nibags added a comment.
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
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19876
To:
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R39:c922bc8352ac: Fix: apply correctly the text colors of the
chosen scheme (authored by nibags).
REPOSITORY
R39
cullmann accepted this revision.
cullmann added a comment.
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 :)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
nibags added a comment.
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)...
REPOSITORY
R39 KTextEditor
REVISION DETAIL
nibags updated this revision to Diff 55004.
nibags marked an inline comment as done.
nibags added a comment.
- Fix attributes in "KDE" and "Vim (dark)" themes
I found a small bug in this patch. In the "KDE" and "Vim (dark)" schemas,
hard colors and text backgrounds, defined in XML files,
cullmann accepted this revision.
cullmann added a comment.
I am ok with this ;=)
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19876
To: nibags, #ktexteditor, #kate, cullmann, mwolff
Cc: mwolff, cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, gennad,
nibags updated this revision to Diff 54775.
nibags added a comment.
- Use QStringLiteral
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19876?vs=54702=54775
BRANCH
fix-schemas
REVISION DETAIL
https://phabricator.kde.org/D19876
AFFECTED FILES
mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
one minor nit, otherwise looks like a good improvement
INLINE COMMENTS
> katehighlight.cpp:78
> +if (schema == QLatin1String("Normal")) {
> +return
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.
Ok with that.
In the long run we should use only the syntaxhighlighting themes and provide
UX for that.
Volunteers?
REPOSITORY
R39 KTextEditor
BRANCH
fix-schemas
REVISION
nibags edited the summary of this revision.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19876
To: nibags, #ktexteditor, #kate, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, gennad, domson,
michaelh, ngraham, bruns, demsking, sars
nibags updated this revision to Diff 54702.
nibags added a comment.
- Pass schema name as parameter
I've also done debug and everything works as it should.
Any problem or detail to change, don't hesitate to say
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
cullmann requested changes to this revision.
This revision now requires changes to proceed.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19876
To: nibags, #ktexteditor, #kate, cullmann
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, gennad, domson,
cullmann added a comment.
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.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
dhaumann added a comment.
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
nibags updated this revision to Diff 54614.
nibags added a comment.
- Add some comments
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19876?vs=54404=54614
BRANCH
fix-schemas
REVISION DETAIL
https://phabricator.kde.org/D19876
AFFECTED FILES
nibags updated this revision to Diff 54404.
nibags added a comment.
- Fix theme names
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
nibags edited the summary of this revision.
nibags added reviewers: KTextEditor, Kate.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D19876
To: nibags, #ktexteditor, #kate
Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham,
bruns, demsking,
nibags created this revision.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
nibags requested review of this revision.
REVISION SUMMARY
BUG: 398758
Since the implementation of KSyntaxHighlighting in KF5.50, KTextEditor always
uses
18 matches
Mail list logo
