ide: introducing themes management #1150
Conversation
This patch adds IDE themes management. It has the same behaviour as QtCreator. There is some default theme provided and the user can copy them to create a new one. Only user created theme can be edited. For now only default theme is provided: "default".
Only background color is used for current line, this patch disables so others options for this entry to improve understanding for users.
There is no difference between General and Syntax categories. This patch consequently removes them.
The bug was introduced by the following patch: ide: themes: remove General and Syntax categories
This patch adds the ability to the theme management to look for a previous custom theme and if available import it into a new theme called "My old theme".
|
I finished to fix my pull request according to Tim comments |
|
in general, it looks good to me. though testing it i came across one issue: post window text and post window emphasis of "my old theme" are imported with black foreground and background. |
|
It seems I can't save the Qt::transparent color. The next time, the color will be loaded, it will be set black. |
This patch modifies the legacy import strategy. Instead of overwrite the default value with the found format, it merge them.
Some keys weren't imported due to a bad group name. This also patch makes sure to erase all legacy theme artefacts from the settings file by removing both highlighting and colors groups.
When introducing the theme management, the current line and post window text formats were modified. This patch restores them to their previous value.
The Qt::transparent color can't be saved into the settings file. When loading this color at the next startup, it is black. This patch avoid to set color background when it is Qt::transparent.
|
Here's the fix for the "black background", coming with some other fixes. |
|
Tim, Did my last patches fixed your problem? Is the branch ok for merge? Do you want me to create a new one with some squashed patchs? |
|
Thanks for this, welcome addition, unobtrusive and useful. I'll start using it and let you know if I find problems. The transparency-problem is not fixed though: As you said "transparent" doesn't get saved, this is still the case. It is applied at first, but after you restart sc, anything set transparent appears black. |
|
Maybe there is an interference with the "inactive editor fade" feature (which ignores the post window)? |
|
It seems that in order to store transparency the the color for the feature must not be defined in sc_ide_conf.yaml. Currently background="000000" is written to the config file. |
|
bagong, It's indeed not possible to save the Qt::transparent value into the yaml file. That's the purpose of this commit: vdonnefort@09a805f. We don't set the format if the color is Qt::transparent. I don't see any problem about it. Let me know the exact steps to reproduce it. About the postwindow grey value, I'm a bit confused: I used the same color as before palette.brush(QPalette::Mid). Maybe I'd better to use Qt::transparent here. |
|
Hum, I've just found how to reproduce the "transparency->black" problem: Modify a foreground of a format which have a transparent background and then restart SC. |
|
Easier: set any color value to transparent and it will be black on restart. |
To match the pre-theme-management appearance, this patch sets the Post Window Text background default value to transparent.
Since the transparent color can't be stored, this patch makes sure that when the picked brush is Qt::NoBrush (~ color transparent), the background is not set. It completes the following commit: ide: theme: fix transparent color
|
Bagong, I just added two patches which must fix problems you reported. Let me know if this seems ok on your side. |
|
Transparency and postwindow behaviour seem okay now. Occasionally it is not possible to open the color-picker - I haven't found a pattern to that yet. And for "current line" it is not possible to change text-color. "Italic" and "bold" also seem to be disfunctional (also if changed manually in sclang_config). Dunno if these later things relate to your PR. |
|
Thanks bagong for your report. This PR is a quite huge project and it's very helpful to have people to test it!
|
|
Bagong, I tested italic and bold for each text format. All non working italic and bold options are non related to my PR. They also belong to "General" syntax, my guess is it is no accident. @timblechmann could you confirm that? Do we want italic and bold options to be also disabled for these text formats? |
|
Any other comments about this PR? :) |
|
sry for being silent, have been touring for the last weeks ... might take a bit that i can have a look at the PR ... but i will |
|
had a look and in general it looks good. it also converted my old theme correctly, though i'd suggest that some people will run this PR to see if their themes will be converted correctly. one small issue: the "line numbers" option shows the wrong background when being "transparent": it uses the background color of the normal text(?), while it should show the background of the "line indicator", which afaict is |
|
Thank you for your review. What is "line indicator" when I set line number to "transparent" background I still see a gray color whereas my normal text background is white. |
|
Any news about this PR? |
|
@telephon @jleben @muellmusik Any news about this PR? |
|
I can't judge it and I can't tell from the discussion if the issues have been resolved? |
|
@telephon, the only remaining opened point is the @timblechmann remark about "line numbers". I may be wrong but the problem is not related to my PR and the whole line numbering thing needs some refactoring. It should have its own PR. I started discussing it with @timblechmann, I hope we will find a good solution. This PM must so be merged without problems. |
|
Okay, in that case I'll merge. |
ide: introducing themes management
|
Hi Vincent, I'm on OS X 10.8.5: |
|
Hi Michael, At the first theme manager usage, the previous configuration is stored into "My old theme". The all elements remains the same. I think you point the wrong commit id (266456b) :) What I understand of your problem is: the "current line" is not correctly imported from the old theme. Is that correct? If you set again the "current line" to black, is it working? |
|
Sorry about the commit confusion! I meant to point at what version I compiled to test. |
|
|
@miczac How do you trig the "current line" colors? :) |
|
Not sure what you mean by "triggering", It's just there: |
|
|
|
The checkbox allows to "remove" the color, to set it to "transparent". |
|
@miczac, this patch vdonnefort@ee832c9 should fix your import problem: some keys (created after 3.6) weren't imported. |
|
@vdonnefort importing from a 3.7a profile created before this PR works now. CurrentLine is there - thx! |
|
Thanks @miczac for testing! Do you have any other problems before I send a PR? |
|
@vdonnefort - if you could wait until evening, something appears weird when switching versions of SC and "updating" then. I got rehearsals now - have to have a deeper look, maybe it's just me. |
|
@vdonnefort - I can see a probably similar situation like you described before about your system: |
|
Thanks again @miczac. This is not related to my patch but I will give a look at it. I updated my branch https://github.com/vdonnefort/supercollider/commits/scide/theme-mgmt-fix with your remarks about the clear foreground button. |
|
@miczac The current line needs the "highlight current line" option set to work (settings > editor > display). I think I can now send the new PR. |
|
@vdonnefort - ah forgot to check. :/ That's it of course. |
This patch adds IDE themes management. It has the same behaviour as QtCreator.
There is some default theme provided and the user can copy them to create a new
one. Only user created theme can be edited. For now only default theme is
provided: "default".