Skip to content
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

PR: Display prompt after errors #4914

Merged
merged 6 commits into from
Aug 22, 2017
Merged

PR: Display prompt after errors #4914

merged 6 commits into from
Aug 22, 2017

Conversation

impact27
Copy link
Contributor

@impact27 impact27 commented Aug 8, 2017

Fixes #4504.

This solves issue #4504 but will sometimes display two in[]

This solves issue spyder-ide#4504 but will sometimes display two `in[]`
@impact27
Copy link
Contributor Author

impact27 commented Aug 8, 2017

screen shot 2017-08-08 at 15 47 30
To clarify:
There is always an extra In[] when an exception occur with this workaround. But at least the last input line is clean.

@ccordoba12 ccordoba12 added this to the v3.2.2 milestone Aug 8, 2017
@ccordoba12
Copy link
Member

@dhirschfeld, what do you think about @impact27's solution?

"""
Reimplemented to reset the prompt if the error comes after the reply
"""
super()._handle_error(msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong. It has to be

super(NamepaceBrowserWidget, self)._handle_error(msg)

Also, please move this addition to ShellWidget because it's not related to the Variable Explorer.

@dhirschfeld
Copy link
Contributor

It seems a bit of a quirky solution - always insert an extra prompt after an exception and sometimes the exception will be printed in that prompt instead of associated with the prompt which caused it.

That said, as a work-around I think it does lead to a better end-user experience as you don't have to escape out of a traceback anymore.

If the underlying issue is with ipykernel perhaps it should be fixed there?

@impact27
Copy link
Contributor Author

It is definitely a work-around. If this is an issue with ipykernel wouldn't it show in every iPython consoles? Do you think we should open a bug report with ipykernel even though this might be related to spyder only?

@ccordoba12 ccordoba12 changed the title Solves issue #4504 PR: Display prompt after errors Aug 14, 2017
@ccordoba12
Copy link
Member

Our tests are failing in PyQt4, but I'm not exactly sure how to fix them...

@impact27
Copy link
Contributor Author

If the error is "AttributeError: 'super' object has no attribute '_handle_error'" I could add a test to check if super has _handle_error? Worst case the msg is not handled, which means the error is not printed. I am not sure how this is happening?

@ccordoba12
Copy link
Member

I could add a test to check if super has _handle_error? Worst case the msg is not handled, which means the error is not printed

I agree with this option, but please do it only for PyQt4. There's a constant in qtpy called PYQT4 that let's you check if Spyder is running with those bindings.

I am not sure how this is happening?

No idea either, but it's not so serious because very few people is using PyQt4 now.

@pep8speaks
Copy link

pep8speaks commented Aug 22, 2017

Hello @impact27! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 22, 2017 at 12:54 Hours UTC

@ccordoba12 ccordoba12 merged commit b0357e9 into spyder-ide:3.x Aug 22, 2017
ccordoba12 added a commit that referenced this pull request Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants