| cullmann | |
| mwolff |
| KTextEditor | |
| Kate |
BUG: 398758
Since the implementation of KSyntaxHighlighting in KF5.50, KTextEditor always uses the "Normal" scheme for text colors.
Previously, the colors were set according to KSyntaxHighlighting::Repository::LightTheme, which corresponds to the "Normal" scheme. Now use KateRendererConfig::global() to get the name of the current selected scheme.
Before:
After:
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Convert theme names from KTextEditor => KSyntaxHighlighting and avoid using invalid KSyntaxHighlighting::Theme objects.
The KDE and vim (dark) themes don't exist in KSyntaxHighlighting, in such cases KSyntaxHighlighting::Repository::LightTheme is used (however, the colors of the original theme are retained).
I think such a workaround is ok, but the real issue here is that KTextEditor does not fully use the KSyntaxHighlighting Theme colors. Instead, it still has its own configuration, such that hacks like this are introduced to somehow make it work. But it's just a matter of time until it breaks again.
Any other comments?
I don't like that one sets the global renderer config.
Where attributesForDefinition is called one knows our schema name. One could just pass it to that routine and do the hack only there.
I've also done debug and everything works as it should.
Any problem or detail to change, don't hesitate to say
Ok with that.
In the long run we should use only the syntaxhighlighting themes and provide UX for that.
Volunteers?
one minor nit, otherwise looks like a good improvement
| src/syntax/katehighlight.cpp | ||
|---|---|---|
| 78 | return QStringLiteral, otherwise you allocate on every function call (also below) | |
I found a small bug in this patch. In the "KDE" and "Vim (dark)" schemas, hard colors and text backgrounds, defined in XML files, don't work. I have updated the diff: now, for these cases, an empty theme will be used and only the attributes defined in the XML files will be applied, such as bold="1" or color="#CCCC".
I have tested this a lot and it works well so far. I don't know if you have any disagreement with the last update (for example, using setTheme() with an empty theme, although I haven't seen this cause any problems)...
I think this is good enough as it is.
I would prefer that we in the future just switch over to use the syntax-highlighting themes, but that is a lot of work :)
Sorry, I committed this diff, but it hasn't been fully accepted. Although I made the changes that mwolff requested. If there is a problem, I can undo the commit or update it....