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: Initial traceback setup to use selected syntax style (IPython Console) #22965

Merged
merged 22 commits into from
Nov 27, 2024

Conversation

dalthviz
Copy link
Member

@dalthviz dalthviz commented Nov 13, 2024

Description of Changes

  • Included a screenshot or animation (if affecting the UI, see Licecap)

Enable context mode tracebacks to follow currently selected syntax style. A preview:

image
image

Issue(s) Resolved

Fixes #22412
Fixes #1052

Notes

Probably the logic here to set the syntax style via the import of the VerboseTB class and set the exception handler to verbose (xmode magic) should go over spyder-kernels and be added as a kernel comm handler method? What do you think @ccordoba12 ?
Requires changes over spyder-kernels: spyder-ide/spyder-kernels#521

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: dalthviz

@dalthviz dalthviz added this to the v6.0.3 milestone Nov 13, 2024
@dalthviz dalthviz self-assigned this Nov 13, 2024
@pep8speaks
Copy link

pep8speaks commented Nov 13, 2024

Hello @dalthviz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-11-27 00:41:13 UTC

@ccordoba12
Copy link
Member

ccordoba12 commented Nov 13, 2024

Probably the logic here to set the syntax style via the import of the VerboseTB class and set the exception handler to verbose (xmode magic) should go over spyder-kernels and be added as a kernel comm handler method?

Yep, I was thinking exactly the same. You should be able to use the ShellWidget.set_kernel_configuration method to do that and then add a new method in spyder-kernels to set the traceback style.

That approach has the great advantage of being backwards compatible because the corresponding handler in spyder-kernels (SpyderKernel.set_configuration) doesn't do anything if it's asked to update a config it can't handle.

Also, since this requires the verbose traceback mode, you need to change that in the kernel too, specifically here:

https://github.com/spyder-ide/spyder-kernels/blob/7b8dc11d487b7f94cf94a9546ea29b65c64e3c84/spyder_kernels/console/start.py#L73

And you also probably need to update the code that allows to click on a traceback filename to open it in the editor due to that.

@dalthviz
Copy link
Member Author

Note: Could it be that this will also end up fixing #1052 ?

@ccordoba12
Copy link
Member

Note: Could it be that this will also end up fixing #1052 ?

Yep, it will! 🎉

 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "75ebcac30"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "75ebcac30"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "589c471ae"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "589c471ae"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
@dalthviz dalthviz changed the title [WIP] PR: Initial traceback setup to use selected syntax style (IPython Console) PR: Initial traceback setup to use selected syntax style (IPython Console) Nov 18, 2024
@dalthviz dalthviz marked this pull request as ready for review November 18, 2024 23:47
@ccordoba12
Copy link
Member

ccordoba12 commented Nov 19, 2024

Thanks @dalthviz for your work on this! A couple of quick comments:

  1. It seems now we need to adjust the ANSI color we're using for the reloaded modules message in the light theme. The light yellow shown in your second screenshot doesn't have enough contrast with respect to the background.

    imagen

  2. The background color used to highlight the line where the error is present in the dark theme is too strong and doesn't allow to read very well the text it highlights:

    imagen

    But according to PR Quick hack to make tb highlighting configurable. ipython/ipython#13756, we should be able to adjust it to use a better color.

@dalthviz
Copy link
Member Author

What colors should be used instead? Something like this works:

image
image

?

 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "f3211825d"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "f3211825d"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "d9e7bad05"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "d9e7bad05"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "6939c42df"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "6939c42df"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "753558ad1"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "753558ad1"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "07b9ca3e8"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "07b9ca3e8"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
@ccordoba12
Copy link
Member

ccordoba12 commented Nov 20, 2024

What colors should be used instead? Something like this works:

  • For the reloaded modules message:
    • Dark theme: The previous yellow (i.e. the one from your initial screenshot) looks good.
    • Light theme: The red you selected in your last screenshot looks fine.
  • For highlighted lines in tracebacks on both themes: I tried a lot of colors, but I found that the red and yellow ones you selected at the beginning were the best ones. Sorry for the noise.

 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "db71627b9"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "db71627b9"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
 --branch=spyder_22965 --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "20f181681"
upstream:
  origin:   "https://github.com/dalthviz/spyder-kernels.git"
  branch:   "spyder_22965"
  commit:   "20f181681"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

One last tiny comment for you @dalthviz, the rest looks good to me.

spyder/plugins/ipythonconsole/widgets/shell.py Outdated Show resolved Hide resolved
dalthviz and others added 3 commits November 26, 2024 17:15
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
…r-kernels.git --branch=master --update --force external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "07f24b6fd"
upstream:
  origin:   "https://github.com/spyder-ide/spyder-kernels.git"
  branch:   "master"
  commit:   "07f24b6fd"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
@dalthviz
Copy link
Member Author

dalthviz commented Nov 26, 2024

Note: Failing tests jobs (linux fast) seem like are being caused due to python/pythondotorg#2663 🤔

@ccordoba12
Copy link
Member

Ok, so we'll have to wait until that issue is solved. In the meantime, please remove your two last commits.

@ccordoba12
Copy link
Member

And things seem to be working again, so we should be fine now.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Great work, thanks @dalthviz!

@ccordoba12 ccordoba12 merged commit 9eb4863 into spyder-ide:master Nov 27, 2024
22 of 34 checks passed
@ccordoba12
Copy link
Member

ccordoba12 commented Nov 27, 2024

@dalthviz, please open a follow-up PR to remove the spyder/plugins/ipythonconsole/utils/style.py module and import it instead from Spyder-kernels.

As we discussed, that's necessary to not have repeated code in two places and it'll be included in 6.1 only.

ccordoba12 added a commit to ccordoba12/spyder that referenced this pull request Nov 27, 2024
…up to use selected syntax style (IPython Console))
ccordoba12 added a commit that referenced this pull request Nov 27, 2024
Backport PR #22965 on branch 6.x (PR: Initial traceback setup to use selected syntax style (IPython Console))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Traceback color handling and definition need improvements Add support for IPython enhanced tracebacks
3 participants