-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: Preserve color scheme on kernel restart #6256
Conversation
self.shellwidget.silent_execute("%colors linux") | ||
else: | ||
self.shellwidget.silent_execute("%colors lightbg") | ||
self.set_console_scheme(self.shellwidget) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change set_console_scheme
to set_color_scheme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a different method called set_color_scheme
. Before submitting the PR, I wasn't sure if I should use it or not because it takes color_scheme
as a parameter, which is a value from the plugin (ipythonconsole.py) and not the widget (client.py). It was a more complicated change, so I opted for directly fixing this one issue.
Do you want me to redo it using the existing set_color_scheme
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please. I think it'd be easier to understand that way. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, will work on it soon.
I think this is good, just left a minor comment. |
# For issue 6235. IPython was changing the setting of | ||
# %colors on windows by assuming it was using a dark | ||
# background. This corrects it based on the scheme. | ||
self.set_color_scheme(sw.syntax_style) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed sw.reset(clear=True)
because it's part of set_color_scheme
.
@ccordoba12 Good suggestions; it's much cleaner now. |
@csabella, now tests for dedicated consoles are failing. |
@ccordoba12 Yes, sorry, trying to figure it out. |
Yay! I'm green! I also tested as much of that as I could think of to do manually. I hadn't thought of the dedicated console, so that was a good test case. :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot Cheryl! Great work here!
Fixes #6235.