PR: Make highlighting colors in the IPython Console match those used in the Spyder Editor #4813
Conversation
|
@jnsebgosselin, thanks a lot for your quick PR! However, this PR needs to be against For next time, I'd like to let you know that we decide which branch to use for each issue by using the Milestone tag (located to the right of every issue/PR). So I tagged issue #4806 with the 3.2.1 milestone. |
|
@ccordoba12 ahhh I see, the Milestone tag, got it. Will check it up next time. |
|
Thanks for understanding!
El 25/07/17 a las 14:38, Jean-Sébastien Gosselin escribió:
…
@ccordoba12 <https://github.com/ccordoba12> ahhh I see, the Milestone
tag, got it. Will check it up next time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4813 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAWS7YCHK2O3jZh2IFxEi4pvTqT6c4Khks5sRkRAgaJpZM4OjAlU>.
|
- text highlight now use the same color as Spyder main application instead of using the light gray color that was hard-coded. - `matched-bracket` highlight is now set to the color defined in Spyder preferences instead of using the hard coded light gray color.
|
You need to rebase your branch onto https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md#changing-base-branch |
|
@rlaverde hehe this is what I'm trying to do but I am not able to push the rebase to the remote. It is rejected with the following error and I'm stuck, trying to find a solution on the world wide web :) Any tips?
|
|
@jnsebgosselin add
|
9ce45c2 to
1c59108
Compare
|
@rlaverde Oh yeah! Many thanks |
|
@jnsebgosselin just bear in mind, that -f, will force push stuff and overwrite the repo, so use carefully :-) (when rebasing) |
|
@dalthviz, please review this one since you were in charge of adding colors to the IPython console. |
| @@ -70,6 +71,9 @@ def __init__(self, ipyclient, additional_options, interpreter_versions, | |||
| # To save kernel replies in silent execution | |||
| self._kernel_reply = None | |||
|
|
|||
| color_scheme = kw['config']['JupyterWidget']['syntax_style'] | |||
| self.set_bracket_matcher_color_scheme(color_scheme) | |||
There was a problem hiding this comment.
Maybe this two lines could be replaced by something like
self.set_bracket_matcher_color_scheme(self.syntax_style)What do you guys think @jnsebgosselin @ccordoba12 ?
Also, please add a comment above, something like # To set the color of the matched parentheses.
There was a problem hiding this comment.
Yes you are right and this works. I didn't see it because it is not obvious where this is inherited.
| @@ -49,7 +49,6 @@ def give_font_style(is_italic): | |||
| in_prompt_color = 'lime' | |||
| out_prompt_color = 'red' | |||
| background_color = color_scheme['background'] | |||
| selection_background_color = '#ccc' | |||
There was a problem hiding this comment.
Why do you have to remove this as part of this PR?
There was a problem hiding this comment.
Sorry, I just saw you mentioned the reason in your description :-)
| @@ -70,6 +71,9 @@ def __init__(self, ipyclient, additional_options, interpreter_versions, | |||
| # To save kernel replies in silent execution | |||
| self._kernel_reply = None | |||
|
|
|||
| # To set the color of the matched parentheses | |||
| self.set_bracket_matcher_color_scheme(self.syntax_style) | |||
There was a problem hiding this comment.
Why is it necessary to add this here? I mean, the call to set_bracket_matcher_color_scheme in set_color_scheme should be enough, shouldn't it?
There was a problem hiding this comment.
set_color_scheme does not seem to be called anywhere on startup. The initial settings are actually passed to the JupytherWidget as argument in the super call and the settings are applied directly within the JupytherWidget. However, these settings do not include the matched parentheses color, since the default hard-coded value is used. That is why we have to set it separately here in the __init__ on our side.
So if we do not add this there, the setting to the matched parentheses color is applied only when there is actually a change to the color scheme after the Spyder is started and a call is made to set_bracket_matcher_color_scheme.
There was a problem hiding this comment.
Ok, got it.
Could you improve your comment above this line with a short summary of what you just said? Else, I'm sure me or someone else will be tempted to remove this line in the future ;-)
There was a problem hiding this comment.
Of course, good idea.
There was a problem hiding this comment.
I'm not sure it is ok. I've tried to keep it as short as possible. What do you guys think?
# Set the color of the matched parentheses here since the qtconsole
# uses a hard-coded value that is not modified when the color scheme is
# set in the qtconsole constructor.
There was a problem hiding this comment.
I think its ok 👍 maybe you could add a reference to the issue, something like See issue 4806. Also, thank you for your help 😄, nice job!
There was a problem hiding this comment.
Yep, that text is descriptive enough, thanks!
|
Thanks @jnsebgosselin, great work! |
Fixes #4806
style.pyis to let the text highlighting color in the IPython console to be the same as for the Spyder main application instead of using the light gray color that was hard-coded.shell.pyis to set the matched-bracket highlighting color to the color defined in Spyderpreferences instead of using the hard coded light gray color.