D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

dhaumann added inline comments. INLINE COMMENTS > kossebau wrote in textcreator.cpp:169 > For the thumbnail the code is rendering on a paper-like canvas, which is > hard-coded as well (see lines above with `QColor ( 245, 245, 245 ); // > light-grey background`). To simulate a print-out, for wha

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added inline comments. INLINE COMMENTS > dhaumann wrote in textcreator.cpp:169 > I saw this review request just now: Using a hardcoded theme is not a good > idea. I suggest to use a solution based on the background color. We have the > following in our example codeeditor: > > setThe

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

dhaumann added inline comments. INLINE COMMENTS > textcreator.cpp:169 > + > syntaxHighlighter.setDefinition(m_highlightingRepository.definitionForFileName(path)); > +const auto highlightingTheme = > m_highlightingRepository.defaultTheme(KSyntaxHighlighting::Repository::Li

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

This revision was automatically updated to reflect the committed changes. Closed by commit R320:d95ff837a9bc: [text thumbnailer] Use KSyntaxHighlighting for text rendering (authored by kossebau). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19432?vs=52901

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

cfeck accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras BRANCH addsyntaxhighlighttotextpreview REVISION DETAIL https://phabricator.kde.org/D19432 To: kossebau, broulik, cfeck Cc: vkrause, cfeck, kde-frameworks-devel, kfm-devel, alexde, fev

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a reviewer: cfeck. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D19432 To: kossebau, broulik, cfeck Cc: vkrause, cfeck, kde-frameworks-devel, kfm-devel, alexde, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanue

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a comment. So, from my side I am fine with the current patch. While the change to QTextDocument (& syntax highlighting?) results in a marginal bigger linespacing and as result up to one line less text rendered in the preview, I find the newer linespacing actually better to rea

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a comment. @vkrause : Any idea why I get `org.kde.ksyntaxhighlighting: Repository got deleted while a highlighter is still active!` with this code? The `KSyntaxHighlighting::SyntaxHighlighter` instance and the `QTextDocument` instance are both created on the stack in the scope

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau updated this revision to Diff 52901. kossebau added a comment. Changes: - clip when drawing the text document, so our "page" margins stay clean - for that reuse vars some more Somehow the old custom margins look nicer to me, so I kept the logic in the code. Seems QTextD

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a comment. New after: F6646824: Screenshot_20190301_144746.png Hm, meh, now I see that `textDocument.drawContents(&painter);` does not stay within the page size the document was given... let's see whether I can do some clipping RE

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau updated this revision to Diff 52899. kossebau added a comment. use original different x & y margins, to gain more chars rendered, as before REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19432?vs=52865&id=52899 BRANCH addsyntaxhighlighttotext

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added inline comments. INLINE COMMENTS > vkrause wrote in textcreator.cpp:169 > I think default background color here refers to what your palette gives you. > The background color defined by highlighting is typically only used for small > text blocks like for alerts ("TODO", "HACK", et

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

vkrause added inline comments. INLINE COMMENTS > kossebau wrote in textcreator.cpp:169 > @vkrause Can you help here and tell what one is intended to use? I think default background color here refers to what your palette gives you. The background color defined by highlighting is typically only u

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a subscriber: vkrause. kossebau added inline comments. INLINE COMMENTS > kossebau wrote in textcreator.cpp:169 > Good idea. Hm, > `highlightingTheme.backgroundColor(KSyntaxHighlighting::Theme::Normal)` > returns black for me, and there is a comment for > `Theme::backgroundColor(

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added inline comments. INLINE COMMENTS > cfeck wrote in textcreator.cpp:169 > KSyntaxHighlighting::Theme also provides a background color. Can that be used > instead of the hardcoded 245? I cannot remember reason why I disabled (or > never enabled) the QPalette code; I suggest to remov

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

cfeck added inline comments. INLINE COMMENTS > textcreator.cpp:169 > + > syntaxHighlighter.setDefinition(m_highlightingRepository.definitionForFileName(path)); > + > syntaxHighlighter.setTheme(m_highlightingRepository.defaultTheme(KSyntaxHighlighting::Repository::LightThem

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau added a comment. Before: F6645792: Screenshot_20190301_000252.png After: F6645790: Screenshot_20190301_03.png There is surely room for more improvements with this thumbnailer, but the syntax

D19432: [text thumbnailer] Use KSyntaxHighlighting for text rendering

kossebau created this revision. kossebau added a reviewer: broulik. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. kossebau requested review of this revision. REVISION SUMMARY Makes previews of text files a bit easier to recognize, both due