implementing improved theming!

[eteran]

implementing improved theming!

[eteran]

Example Light Theme:

[Theme]
theme.palette.alternatebase.disabled=#ffe3e5e7
theme.palette.alternatebase=#ffeff0f1
theme.palette.base.disabled=#fff0f0f0
theme.palette.base=#fffcfcfc
theme.palette.brighttext.disabled=#ffffffff
theme.palette.brighttext=#ffffffff
theme.palette.button.disabled=#ffe3e5e7
theme.palette.button=#ffeff0f1
theme.palette.buttontext.disabled=#ffa0a1a2
theme.palette.buttontext=#ff232627
theme.palette.dark.disabled=#ff82878c
theme.palette.dark=#ff888e93
theme.palette.highlight.disabled=#ffe3e5e7
theme.palette.highlight=#ff3daee9
theme.palette.highlightedtext.disabled=#ffa0a1a2
theme.palette.highlightedtext=#fffcfcfc
theme.palette.light.disabled=#ffffffff
theme.palette.light=#ffffffff
theme.palette.link.disabled=#ffa2c8e0
theme.palette.link=#ff2980b9
theme.palette.linkvisited.disabled=#ffc7cbcb
theme.palette.linkvisited=#ff7f8c8d
theme.palette.mid.disabled=#ffbbc0c5
theme.palette.mid=#ffc4c8cc
theme.palette.midlight.disabled=#ffebedee
theme.palette.midlight=#fff6f7f7
theme.palette.shadow.disabled=#ff474a4c
theme.palette.shadow=#ff474a4c
theme.palette.text.disabled=#ffa8a9a9
theme.palette.text=#ff232627
theme.palette.tooltipbase.disabled=#ff232627
theme.palette.tooltipbase=#ff232627
theme.palette.tooltiptext.disabled=#fffcfcfc
theme.palette.tooltiptext=#fffcfcfc
theme.palette.window.disabled=#ffe3e5e7
theme.palette.window=#ffeff0f1
theme.palette.windowtext.disabled=#ffa0a1a2
theme.palette.windowtext=#ff232627

Example Dark Theme:

[Theme]
theme.palette.alternatebase.disabled=#ff2f3338
theme.palette.alternatebase=#ff31363b
theme.palette.base.disabled=#ff212427
theme.palette.base=#ff232629
theme.palette.brighttext.disabled=#ffffffff
theme.palette.brighttext=#ffffffff
theme.palette.button.disabled=#ff2f3338
theme.palette.button=#ff31363b
theme.palette.buttontext.disabled=#ff6e7275
theme.palette.buttontext=#ffeff0f1
theme.palette.dark.disabled=#ff1b1e21
theme.palette.dark=#ff1d2023
theme.palette.highlight.disabled=#ff2f3338
theme.palette.highlight=#ff3daee9
theme.palette.highlightedtext.disabled=#ff6e7275
theme.palette.highlightedtext=#ffeff0f1
theme.palette.light.disabled=#ff444b51
theme.palette.light=#ff464d54
theme.palette.link.disabled=#ff234257
theme.palette.link=#ff2980b9
theme.palette.linkvisited.disabled=#ff404648
theme.palette.linkvisited=#ff7f8c8d
theme.palette.mid.disabled=#ff292d32
theme.palette.mid=#ff2b3034
theme.palette.midlight.disabled=#ff3a4046
theme.palette.midlight=#ff3c4248
theme.palette.shadow.disabled=#ff141618
theme.palette.shadow=#ff151719
theme.palette.text.disabled=#ff65686a
theme.palette.text=#ffeff0f1
theme.palette.tooltipbase.disabled=#ff31363b
theme.palette.tooltipbase=#ff31363b
theme.palette.tooltiptext.disabled=#ffeff0f1
theme.palette.tooltiptext=#ffeff0f1
theme.palette.window.disabled=#ff2f3338
theme.palette.window=#ff31363b
theme.palette.windowtext.disabled=#ff6e7275
theme.palette.windowtext=#ffeff0f1

Either one goes in ~/.config/codef00.com/edb.conf to test

[eteran]

LOL, yea I agree. I plan to update the whole travis pipeline to use a newer ubuntu (xenial?) and it'll have I think GCC 8 by default.

Only downside is that the newer ubuntu comes with newer Qt. So until I'm done with this PR, I'll stick with th ability to test against the Qt version we claim to support 🙂.

But once themes are in, I plan to pull the trigger on c++17, Qt 5.9, etc... Which will update our entire build requirements.

[eteran]

@10110111 Also, I did entirely forget that when you asked about gcc-4.8. While that is the version that TravisCI defaults to for trusty images. We do override that with a gcc-5 compiler.

Your point is still 100% correct, but we do require gcc-5 to at a minimum because we use c++14.

[10110111]

You need to set CXX_STANDARD_REQUIRED property to ON to make CMake check that CXX_STANDARD option is actually in effect for the current compiler (there's a rather stupid default behavior IMO). Then the error messages will come from CMake itself, rather than from the inadequate GCC.

[10110111]

theme.palette.alternatebase.disabled=#ffe3e5e7

Does it actually make any sense to specify alpha here? Maybe better restrict the format to #rrggbb?

[10110111]

May be better to warn about this beforehand, e.g. in the label:

Theme (takes effect on EDB restart): [ System ⌄]

Then no modality will interfere with the user's actions.

[eteran]

That's easy enough. Everything seems to work correctly besides that I hope?

[10110111]

Seems to work. But... can we add a "display name" for a theme? It's not too nice to have the filename like mytheme.ini as theme name in the combobox, along with "System", "Dark [Built-in]", "Light [Built-in]". Maybe something like

[Name]
My super great theme
[Theme]
theme.address.foreground=#335577

[eteran]

theme.palette.alternatebase.disabled=#ffe3e5e7

Does it actually make any sense to specify alpha here? Maybe better restrict the format to #rrggbb?

So I literally just copied the values from KDE's default themes. I think it does no real harm to allow the alpha channel since we can just accept "anything that can be passed to QColor's constructor. I agree it probably won't have much use... but who knows :-P.

Seems to work. But... can we add a "display name" for a theme? It's not too nice to have the filename like mytheme.ini as theme name in the combobox, along with "System", "Dark [Built-in]", "Light [Built-in]".

I had the same thought, but there are complexities involved.

1. what if two themes have the same name
2. what if they choose to name their theme "System" or otherwise find a way to conflict with built-in themes

To solve this, maybe allow them to specify the name in the file, but we display it in the combobox like:

"My super great theme - mytheme.ini" ?

[10110111]

what if two themes have the same name

Just ignore this case. Show them as two themes with the same name, and let the user cope with it. For EDB this shouldn't be a problem, since the display name would be used only for display, not for saving in the config file. At least I don't see much benefit in using theme names as handles.

what if they choose to name their theme "System" or otherwise find a way to conflict with built-in themes

This is also fixed by the above approach.

[eteran]

@10110111

OK, now there is an optional section called [Meta] where you can specify the display name of the theme, for example:

[Meta]
name=My super great theme
[Theme]
theme.address.foreground=#335577

If that field is present, then it will display that name to the user (internally it is still tracking the filename). If it isn't present, then it will fall back on using the filename as before.

Let me know what you think!

[eteran]

@10110111 I think we're in good shape at this point. I really appreciate the feedback!

Since this is a fairly large change, I won't merge until I get the go-ahead from you (especially given that you've already caught several small things!).

[10110111]

A convenience thing: may be a good idea to create the empty directory <config-dir>/themes when creating/updating edb.conf, so that the users had no doubt on where to put their themes.

[eteran]

That shouldn't be too hard to do... lemme see if I can cook that up real quick :-).

[10110111]

[Theme]
theme.palette.alternatebase.disabled=#ff2f3338
theme.palette.alternatebase=#ff31363b
theme.<...>
theme.<...>

Do we really need these theme. prefixes? All these keys are already in the [Theme] section, can't they be shortened?

[eteran]

directory creation is done. Yea, you're right, we don't need the "theme" prefix. That was a holdover from when the theme stuff was in the edb.conf file.

I can make it shorter :-).

[eteran]

OK, "theme." prefix is gone :-)

[10110111]

Do you think the following format would be easier to read, compose, and edit? (I'm dropping the Theme token, but it might be put in the section names, although I don't think it's useful in a theme file.)

[Meta]
name=Example theme

[Address]
foreground=#ba86c0

[AlternatingByte]
foreground=grey

[Arithmetic]
background=transparent
foreground=#569cd6
italic=false
underline=false
weight=50

[Badge]
background=blue
foreground=white

[Brackets]
background=transparent
foreground=#b5cea8
italic=false
underline=false
weight=75

[Comma]
background=transparent
foreground=#b5cea8
italic=false
underline=false
weight=75

[Comparison]
background=transparent
foreground=#569cd6
italic=false
underline=false
weight=50

[Constant]
background=transparent
foreground=#ce7b48
italic=false
underline=false
weight=50

<...>

[eteran]

Mixed feelings on the having a section per thing idea.

It won't make the file any shorter in practice and a theme author can always use newlines to space things out if they prefer.

[10110111]

My main concern is the repetition of prefixes. They clutter the file, making it a bit harder to follow.

[eteran]

Yea, that's not an unreasonable concern.

Part of me likes the CSS-ish notation, and also honestly themes are on average write once kinda files. Most users are just gonna copy existing files, and maybe change a value here and there.

But I do agree that your suggestion makes things slightly more readable...

Lemme think about it.

[eteran]

@10110111 I gotta be honest, when I look at the two options here, I just like the <object>.<property>=<value> syntax. Obviously, this is boiling down to a matter of preference.

I can say that projects such as x64dbg seem to have a healthy theme ecosystem, and have an even harder to read syntax ;-) (they use something like mine but with no visual separators in the names):

https://github.com/Gbps/x64dbg-consonance-theme/blob/master/consonance.ini

So I don't think either one would be a deal-breaker for users.

[10110111]

OK then. I think current state is good enough for master.

[eteran]

Woot! Awesome, thanks again for your input!