PR: Don't apply color schemes that require a restart and fix other issues about them #18831
Conversation
|
Hello @ccordoba12! 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 2022-08-01 21:53:34 UTC |
ccordoba12
changed the title
There was a problem hiding this comment.
Thanks @ccordoba12 ! Checking locally seems like now you can't apply some of the possibilities mixing the light/dark themes with ligh/dark syntax themes. For example, selecting the Light theme when having previously Automatic and the Spyder Dark syntax theme causes no change or dialog to show up:
Also, if you then change from Light to Automatic you get the restart dialog but the syntax theme gets applied too:
Other than that seems like the changes are working as expected 👍
ccordoba12
force-pushed
the
dont-apply-color-scheme-on-restart
branch
from
0910c88
to
ed4b931
Compare
ccordoba12
changed the title
|
Thanks for the review @dalthviz! I think I fixed the error you reported above, so this should be ready for another review. I also took the opportunity to fix other bugs about color schemes that were bothering me as well. Please see the updated description above to learn about them. |
There was a problem hiding this comment.
Thanks @ccordoba12 ! I checked locally and seems like the previous behavior was fixed 👍. I left some comments and a suggestion for a comment but other than that this LGTM 👍
The problems were caused by using the options saved on disk instead of the ones selected on the interface.
This will apply a color scheme that (i) the user selected along with a UI theme that required a restart; and (ii) it can be applied when they select another UI theme that doesn't require a restart.
This allows color schemes to be applied correctly after switching them.
- This was the missing extra selection to update so new color schemes appear to users as expected. - Also rehighlight extra selections after updating a color scheme only if the corresponding option is enabled.
ccordoba12
force-pushed
the
dont-apply-color-scheme-on-restart
branch
from
ed4b931
to
d3a6119
Compare
Description of Changes
Don't apply new color schemes if they require a restart. Instead, simply show users the restart message.
Right now users end up in this situation when changing to a light color scheme, which is confusing:
With this PR we only show the restart message, which is clearer according to this Printing from the editor shows dark background or blank text #2521 (comment)
Simplify the restart message to not mention the options that require it (see the second screenshot above). I think that's an implementation detail that users don't need to know.
Update occurrences, line and cell selections in the editor when applying a new color scheme. This allows color schemes to be applied correctly after switching them.
Before
After
Add new API methods to disable/restore notifications for options.
Make plugins react to UI theme option changes. This applies the color scheme correctly under some combinations of color scheme and UI theme option changes.
Make strings related to UI theme options translatable.
Fix a couple of small issues reported by our linter in the Pylint plugin (I noticed them when working on this PR).
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: @ccordoba12