X Tutup
The Wayback Machine - https://web.archive.org/web/20200905014529/https://github.com/bpython/bpython/pull/485
Skip to content

/ bpython

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to enable debugging on uncaught exceptions. #485

Open
wants to merge 4 commits into
base: master
from
Open

Add feature to enable debugging on uncaught exceptions. #485

wants to merge 4 commits into from

Conversation

@kdart
Copy link

kdart commented Feb 18, 2015

The debugger is selected by an environment variable, PYTHON_DEBUGGER. This
should be set to a module path. If it's not set it will use the standard pdb.

Press F12 to toggle the feature off and on. When off the original behaviour is
used.

Also add pretty-printing option. Currently this is only selected when invoked
from the CLI.
@kdart
Add feature to enable debugging on uncaught exceptions.
Loading status checks…
b1f0afc 
Also add pretty-printng option.
The debugger is selected by an environment variable.
@sebastinas
Copy link
Contributor

sebastinas commented Feb 18, 2015

Thank you for implementing this feature. I'll add some comments to the code once I've had the time to play with it a bit.

The test suite now fails. This needs to be fixed.

I'm afraid pretty printing won't make it into bpython. See #34 for details. But I'm happy to merge bits for the documentation explaining the use of PYTHONSTARTUP.
bpython/config.py Outdated
@@ -119,7 +119,8 @@ def loadini(struct, configfile):
'transpose_chars': 'C-t',
'undo': 'C-r',
'up_one_line': 'C-p',
'yank_from_buffer': 'C-y'
'yank_from_buffer': 'C-y',
'debug': 'F12',

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 18, 2015

Contributor

Please keep the entries sorted by key.
bpython/cli.py
@@ -160,6 +161,7 @@ def __init__(self, interface):
self.encoding = getpreferredencoding()
self.interface = interface
self.buffer = list()
self.errors = "strict"

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 18, 2015

Contributor

Why? Does this fix a bug? If so, this should be a separate commit in a separate pull request.

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 19, 2015

Author

I forget now... I actually made these original changes a long time ago. I think was related to getting it to work with Python 3. It could probably be removed, but I kept it just in case there was still a reason for it. It makes stderr work like stdout.

Python> sys.stderr.errors
'backslashreplace'
Python> sys.stdout.errors
'strict'

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Mar 26, 2015

Contributor

It should still be a separate commit. Please remove it. It has nothing to do with the new feature.
bpython/debugger.py Outdated

def _get_debugger():
global post_mortem
modname = os.environ.get("PYTHON_DEBUGGER", "pdb")

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 18, 2015

Contributor

This should default to bpdb.

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 19, 2015

Author

Ok. But that one doesn't always get back to the original bpython, and can also fail if you don't have all the dependencies (which I don't have because the curses variant doesn't need them).
bpython/debugger.py

post_mortem = None

def _get_debugger():

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 18, 2015

Contributor

The whole function lacks proper error handling. What if a module named modname does not exist? What if mod.pm does not exist?

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 19, 2015

Author

It will raise an ImportError so that the user (who presumably knows what he/she is doing if they are going to set PYTHON_DEBUGGER) can fix it.

Only "post_mortem" is looked for. "pm" is a local variable.

Sometimes it's best to just let errors pass up, so the user can get a full stack trace and know where to fix it.

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 19, 2015

Contributor

No, crashing because the user typoed PYTHON_DEBUGGER is not an option. The whole session would be lost.

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 19, 2015

Author

You mean if the typoed the value of PYTHON_DEBUGGER. They will lose their session once and then fix the problem. However that can be easily fixed I guess and write out an error message.

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 20, 2015

Author

Fixed now. The echo method is used to print a warning if the debugger module couldn't be imported earlier.
bpython/debugger.py
from __future__ import absolute_import
from __future__ import print_function
from __future__ import unicode_literals
from __future__ import division

This comment has been minimized.

Sign in to view
@sebastinas

sebastinas Feb 18, 2015

Contributor

It looks like they are not need here.

This comment has been minimized.

Sign in to view
@kdart

kdart Feb 19, 2015

Author

Standard header for my code, for better Python 3 compatibility.
@kdart
Copy link
Author

kdart commented Feb 19, 2015

Well, it worked for me. :) What is the error? I see it didn't like the -p flag. But that's being removed now.
kdart added 2 commits Feb 19, 2015
@kdart
Remove pretty print feature. make bpdb the default.
Loading status checks…
bfa6798
@kdart
Make the debugger selection issue a warning if it could not be loaded.
Loading status checks…
f5bb13d
@kdart
Copy link
Author

kdart commented Mar 22, 2015

Any other issues with this request?
@sebastinas
Copy link
Contributor

sebastinas commented Mar 26, 2015

Support in the curtsies frontend is missing.
@thomasballinger
Copy link
Member

thomasballinger commented Apr 13, 2015

This is a cool feature! I'm trying to think about how it should work with the curtsies frontend.
@thomasballinger
Copy link
Member

thomasballinger commented Apr 13, 2015

UI-wise, it'd be nice to have messages similar to when auto-reloading is turned on.

There are some unicode issues atm, but fixing these seems doable.

I wonder if the debugging should happen in an alternate screen (so history of it is erased) or inline with the rest of the session?
@thomasballinger
Copy link
Member

thomasballinger commented Apr 13, 2015

I've got this nominally working in the Curtsies frontend, but going back into bpython doesn't work - the Curtsies frontent can't be launched from within itself, it doesn't like the fake stdin/out/err. A workaround would be to embed with the curses frontend, but that doesn't seem great for going forward. This is a problem with bpdb and Curses though, not this PR.

See what you think of https://github.com/bpython/bpython/tree/debugging-uncaught-exceptions, it's not probably ready yet but I'd like to get it up to snuff and merge without requiring getting launching bpython from bpdb working, that should be another bug.
@kdart
Copy link
Author

kdart commented Apr 14, 2015

Sorry for the delay. I haven't used Curtsies before so I needed some time to look into it. Frankly, still not sure I like it. The idea seems OK, though. But is it really needed? The curses UI is still faster, and it works.
@sebastinas
Copy link
Contributor

sebastinas commented Apr 14, 2015

The curses UI will be removed sooner or later. It is no longer maintained.
@thomasballinger
Copy link
Member

thomasballinger commented Apr 15, 2015

Blocked on #520, once in bpdb bpython-curtsies can't be run yet. kdart make with work for bpython-curses with https://github.com/bpython/bpython/pull/485/files#diff-15c8a80fae5902b7d458888b6149c023R1967, I think I can figure something similar out.
@thomasballinger
Copy link
Member

thomasballinger commented Apr 15, 2015

I've tried to allow multiple bpython sessions by making the terminal changing objects global, see https://github.com/bpython/bpython/compare/debugging-uncaught-exceptions

I'm not sure how I feel about this. It's a hack, and history in the new bpython session isn't preserved. There are some details with undo in certain cases that I haven't worked out yet.

I'd also like to have the debugger turned off once this new bpython session starts - I think it gets confusing when the second (debugging) bpython session has an error and a second pdb is opened up.

This may be too complicated to be worth it - better might be to disallow bpython from being called, and if a session is already going on making the B command in bpdb a nop.

I'd also prefer a solution that would allow pudb to be used as a debugger, but I think that would require using a ptty or something as drastic.
@thomasballinger
Copy link
Member

thomasballinger commented Apr 15, 2015

@kdart Any advice on getting pycopia working? I'm trying to run it locally to test that the debugger works but having trouble with pyrex versions (I think).
@thomasballinger
Copy link
Member

thomasballinger commented Apr 15, 2015

I'd be in favor of taking this PR and making pdb the default debugger for now, and pushing off getting bpython working in bpython. This would mean making the Curtsies objects global, just doing something like https://github.com/bpython/bpython/compare/simple-auto-debugging
@kdart
Copy link
Author

kdart commented Apr 16, 2015

@thomasballinger Hello. Thanks for checking it out. The setuptools package used to automate the building of pyx files using pyrex or cython. But now it doesn't seem to do that. You can do it manually first, for now.

$ cd src/pycopia/utils
$ cython pycopia.itimer.pyx
$ python2.7 setup.py build

The build and install should work fine once that is done.
@kdart
Merge branch 'master' of https://github.com/bpython/bpython
Loading status checks…
a40018f
@thomasballinger thomasballinger mentioned this pull request Nov 18, 2016
Closed
@richin13
Copy link

richin13 commented Apr 1, 2019

Is this ever going to make it? 😢
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@kdart @sebastinas @thomasballinger @richin13
You can’t perform that action at this time.
X Tutup