D18663: KateSpellCheckDialog: Remove action "Spellcheck Selection"

dhaumann added a comment. In general I am ok eith this. Although it's a tiny bit less transparent to the user. INLINE COMMENTS > spellcheckdialog.cpp:74 > { > -spellcheck(m_view->cursorPosition()); > +if (m_view->selection()) { > +spellcheckSelection(); I don't like the fac

D17932: Improvements to completion

dhaumann added a comment. Well, giving it a try and merge should be postponed ubtil tomorrow: dfaure tags today, so we would then have 1 month of internal testing. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17932 To: thomassc, #ktexteditor, #kdevelop, mwolff

D18509: Fixed Bug Which Caused Infinite Loop While Highlighting Kconfig Files

dhaumann added a comment. Related bug report: https://bugs.kde.org/show_bug.cgi?id=403215 REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D18509 To: jwilbee, #framework_syntax_highlighting, cullmann, dhaumann Cc: kde-frameworks-devel, nibags, kwrite-devel,

D18714: Add some Maxima constans and function

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me, the only missing part is to increase thw "version" number each time a highlighting file is changed. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DE

D18714: Add some Maxima constans and function

dhaumann added a comment. In your branch, you should type: `arc land` This will merge your branch to master and commit/push your changes, and close this revision. This only works, if you have commit access. If not, we'll take care of this for now. REPOSITORY R216 Syntax Highlighting BRA

D18116: Add TODO KF6 note

dhaumann added inline comments. INLINE COMMENTS > document.h:394 > +// TODO KF6: Not needed anymore since we show load trouble as > KTextEditor::Message. > +// Remove all code which set m_openingErrorMessage > bool openingError() const; Hm, is it really that `bool openingError

D18116: Add TODO KF6 note

dhaumann accepted this revision. REVISION DETAIL https://phabricator.kde.org/D18116 To: loh.tar, #ktexteditor, cullmann, dhaumann Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D18734: Update octave.xml to Octave 4.2.0

dhaumann added a comment. Could you also switch to WordDetect for the trivial cases, and set kateversion="5.0" ? INLINE COMMENTS > octave.xml:2690-2711 > beginRegion="block" /> > endRegion="block" /> > beginRegion="block" /> > endRegion="block" /> >

D18734: Update octave.xml to Octave 4.2.0

dhaumann added a comment. ...And there are also new Default Styles for kateversion="5.0", see: https://kate-editor.org/2014/03/07/kate-part-kf5-new-default-styles-for-better-color-schemes/ Maybe you could also use these for the itemData, like for Operator dsOperator, or for Builtin dsBui

D18116: Add TODO KF6 note

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:574b08d08111: Add TODO KF6 note (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https:

D18163: Set the color scheme to Printing for Print Preview

This revision was automatically updated to reflect the committed changes. Closed by commit R39:28e59740d070: Set the color scheme to Printing for Print Preview (authored by ahmadsamir, committed by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1

D18163: Set the color scheme to Printing for Print Preview

dhaumann edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D18163 To: ahmadsamir, cullmann, #ktexteditor, dhaumann, mwolff Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D18788: Refactor KateViewInternal::mouseDoubleClickEvent(QMouseEvent *e)

dhaumann added a comment. I am with Kåre here: this change does not fix any issue nor does it improve the current state significantly. In fact, there is a risk of introducing a regression by removing the accept/reject calls. I don't think this should go in as is. Shubham, on bugs.kd

D18846: allow to compile without Q_FOREACH support

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D18846 To: cullmann, vkrause, dhaumann Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, cullmann, sars

D18846: allow to compile without Q_FOREACH support

dhaumann added a comment. Btw, could we add a cmake definition to disallow foreach? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D18846 To: cullmann, vkrause, dhaumann Cc: kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns, demsking, cullmann

D18864: FlatBuffers schema syntax highlighting

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Hi Harald, nice hearing from you. Patch looks good and can almost go in as is. But could you address the two comments and also add a test file for unit testing? It can be short, is not

D18788: Refactor KateViewInternal::mouseDoubleClickEvent(QMouseEvent *e)

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I think it's ok, although it is arguably whether this improves anything. Still, let's move on. REPOSITORY R39 KTextEditor BRANCH arcpatch-D18788 REVISION DETAIL https://phabrica

D18891: Avoid leaking a QObject in ExternalExtractor

dhaumann added a comment. ThT looks reasonable, if ExtractorPlugin even has `QObject * parent = nullptr` I'd even remove nullptr as well. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D18891 To: bruns, #baloo, #frameworks, ngraham, poboiko, astippich Cc: dhau

D18883: Add PDF thumbnailer

dhaumann added inline comments. INLINE COMMENTS > pdfcreator.cpp:62 > +// Compute a dpi that results in a thumbnail roughly the size we > requested > +const qreal fakeDpiX = width / page->pageSizeF().width() * 72.0; > +const qreal fakeDpiY = height / page->pageSizeF().height() * 72.0

D18894: [RFC] General config interface

dhaumann added a subscriber: mwolff. dhaumann added a comment. This approach spreads they key "Word Wrap Column" across many files. You have to know the key and avoid typos. Currently we have this hard-coded and therefore statically checked by the compiler, which is very good. In grneral

D18864: FlatBuffers schema syntax highlighting

dhaumann accepted this revision. REPOSITORY R216 Syntax Highlighting BRANCH flatbuffers (branched from master) REVISION DETAIL https://phabricator.kde.org/D18864 To: harald, dhaumann, cullmann Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, gennad, michaelh, ngraham, bruns, d

D18895: AsciiDoc: Add folding for sections

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I think this change is ok. Although, adding many RegExprs is slow compared to other rules. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D18895 T

D18895: AsciiDoc: Add folding for sections

This revision was automatically updated to reflect the committed changes. Closed by commit R216:54ac6d34a339: AsciiDoc: Add folding for sections (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D18895?vs=51314&id=51442#toc REPOSITORY R216 Syntax Highlighting CHANG

D18982: prevent assertion in regex load

dhaumann added a comment. I think that makes a lot of sense. Maybe we should even always print the warning, not only in debug mode? See comment below... PS: When submitting patches without phabricator's arc tool, could you add more diff context, like `git diff -U99 ...`. Then reviewing i

D18997: Add functions returning all definitions for a mimetype or file name

dhaumann added inline comments. INLINE COMMENTS > repository.h:176 > + */ > +QVector definitionsForFileName(const QString &fileName) > const; > + As I understand, this currently is unsorted. Wouldn't it make sense to sort the returned vector by priority? This information is also missi

D19001: katetextbuffer: refactor TextBuffer::save() to better separate code paths

dhaumann added a comment. In D19001#413694 , @cullmann wrote: > This looks reasonable, but could we get some Qt/KDE style CamelCase enum values? > > enum SaveResult { > > SAVE_FAILED = 0, > SAVE_NO_PERMS, > SAVE_SUCC

D18997: Add functions returning all definitions for a mimetype or file name

dhaumann added a comment. Ping? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D18997 To: davschul, vkrause, cullmann Cc: dhaumann, cullmann, kwrite-devel, kde-frameworks-devel, gennad, bmortimer, michaelh, genethomas, ngraham, bruns, demsking, vkrause,

D19161: Use QTextFormat::TextUnderlineStyle instead of QTextFormat::FontUnderline

dhaumann added a comment. This patch looks good to me, even though I cannot reproduce the issue following the steps in https://bugs.kde.org/show_bug.cgi?id=399278. But yes, the Qt documentation says to use TextUnderlineStyle instead of FontUnderline. The implementation of fontUnderline()

D19161: Use QTextFormat::TextUnderlineStyle instead of QTextFormat::FontUnderline

dhaumann added a comment. Btw, searching in lxr.kde.org for FontUnderline reveals some more hits: https://lxr.kde.org/ident?_i=FontUnderline&_remember=1 REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19161 To: ahmadsamir, #ktexteditor, cullmann, dhaumann Cc: kde

D19170: Fix crash while moving files

dhaumann added inline comments. INLINE COMMENTS > kjob.cpp:105 > > +if (isAutoDelete()) { > +deleteLater(); Alternatively to moving the code, you could also use a `QPointer that(this);` to monitor whether the QObject is still existing. Then, you'd have to call if (that && isAu

D19170: Fix crash while moving files

dhaumann added a reviewer: dfaure. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D19170 To: hallas, #frameworks, elvisangelaccio, dfaure Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D19168: Fix crash in Dolphin when dropping trashed file in trash

dhaumann added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D19168 To: hallas, #frameworks, elvisangelaccio, ngraham, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19200: Return sorted definitions for file names and mime types

dhaumann added inline comments. INLINE COMMENTS > repository.cpp:83 > { > -if (candidates.isEmpty()) > -return Definition(); > - > -std::partial_sort(candidates.begin(), candidates.begin() + 1, > candidates.end(), [](const Definition &lhs, const Definition &rhs) { > +std::pa

D19200: Return sorted definitions for file names and mime types

dhaumann added a comment. Cool, thanks! ...which now leads us to the point where the only missing part is a unit test for the two new public functions :-) REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19200 To: davschul, cullmann, vkrause Cc: dhaumann,

D19249: Improve highlighting of TypeScript (and React) and add more tests for PHP

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH update-ts REVISION DETAIL https://phabricator.kde.org/D19249 To: nibags, #framework_syntax_highlighting, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel,

D19258: Do not print indent lines

dhaumann created this revision. dhaumann added a reviewer: loh.tar. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY Avoid printing indentation lines when "[x] Show indentation line

D19254: [RFC] KateRenderer: Show tabs and space characters the smart way

dhaumann added a comment. I think this, since it removes visual clutter. I think this should be configurable... Question is how to properly add an option in the config widget, since in the code I can see that this feature changes several options: tabs + trailing spaces + nonprintable ch

D19263: Add more highlighting for nested languages in markdown.

dhaumann added a comment. Could you extend autotests/input/test.markdown ? Then call ./autotests/update-reference-data.sh in the build folder to copy the changes into the src folder, and update this patch. PS: And increase the "version" in the langauge xml element at the beginning of t

D19193: KateSearchBar: Show also search has wrapped hint in nextMatchForSelection() aka Ctrl-H

dhaumann added a comment. Looks good to me - thanks. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19193 To: loh.tar, #ktexteditor Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, ngraham, bruns, demsking, cullmann, sars

D19193: KateSearchBar: Show also search has wrapped hint in nextMatchForSelection() aka Ctrl-H

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19193 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, ngraham, brun

D19193: KateSearchBar: Show also search has wrapped hint in nextMatchForSelection() aka Ctrl-H

This revision was automatically updated to reflect the committed changes. Closed by commit R39:7d9bc72a1bb1: KateSearchBar: Show also search has wrapped hint in nextMatchForSelection() aka… (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.o

D19263: Add more highlighting for nested languages in markdown.

dhaumann added a comment. Looks good to me, thanks. Before committing, could you increase the kateversion="5.0" in the language xml element as well? Otherwise KDE 4 users could get this via the download dialog as well, without getting the depending files due to 5.0 requirements in the o

D19263: Add more highlighting for nested languages in markdown.

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH nested (branched from master) REVISION DETAIL https://phabricator.kde.org/D19263 To: vandenoever, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-deve

D19193: KateSearchBar: Show also search has wrapped hint in nextMatchForSelection() aka Ctrl-H

dhaumann added a comment. >> I think that may also fix the case example in https://bugs.kde.org/show_bug.cgi?id=398731 > > @dhaumann No comment to this? @cullmann May you? Yes, that is a good idea. The only issue I see is that with Ctrl+H you don't have a clear starting point. May

D19258: Do not print indent lines

This revision was automatically updated to reflect the committed changes. Closed by commit R39:b11afe5a1438: Do not print indent lines (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D19258?vs=52389&id=52434#toc REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDAT

D19263: Add more highlighting for nested languages in markdown.

dhaumann added a comment. Well, if it's fixed by adding the missing reference update then please do so. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19263 To: vandenoever, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-devel, kde-framewo

D19286: Add unit test for definitionsForFileName() and definitionsForMimeType()

dhaumann created this revision. dhaumann added reviewers: cullmann, davschul. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY Add unit tests for newly added functions for KF 5.56.

D19215: Install DefinitionDownloader header

dhaumann added a comment. I also do not see an issue with this. But I would like to add some background info that may be interesting: Downloading the syntax highlighting files will download all files, and not just the ones that changed etc. This by itself is fine, since it guarantees co

D19286: Add unit test for definitionsForFileName() and definitionsForMimeType()

This revision was automatically updated to reflect the committed changes. Closed by commit R216:41f4e86ee522: Add unit test for definitionsForFileName() and definitionsForMimeType() (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D19286?vs=52471&id=52477#toc REPOSIT

D19215: Install DefinitionDownloader header

This revision was automatically updated to reflect the committed changes. Closed by commit R216:df18fada76b2: Install DefinitionDownloader header (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19215?vs=52269&id=52478 REVISI

D19263: Add more highlighting for nested languages in markdown.

dhaumann added a comment. Hi Jos, could you please check again the output of the highlighting indexer? ContextChecker::check: "/home/dh/kde/kf5/src/frameworks/syntax-highlighting/data/syntax/markdown.xml" Unused contexts: QSet("kconfig-code") The checker says that this context is

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Please add the comment as suggested by Thomas. REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D19283 To: cullmann, dhaumann Cc: loh.tar, th

D19263: Add more highlighting for nested languages in markdown.

dhaumann added a comment. Good idea, thanks. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19263 To: vandenoever, #framework_syntax_highlighting, dhaumann Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, dem

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

dhaumann added a comment. We trade one bug for another. Which one is worse? Next Frameworks Tag is on Saturday, 2nd, i.e. in 2 days. Revert, or give it a try? PS: This is never going to be fixed, since some lines (in this case with emojis, but could also be bidi text or arabic lette

D19416: Restore the search wrapped message to its former type and position.

dhaumann added subscribers: neundorf, dhaumann. dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. @neundorf With this we are back to where we came from. Interesting to see how history repeats, and repeats, and repeats... Accepting this

D12336: Replace several Q_ASSERTs with proper checks

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I am very much in favour of this change. In fact, we had this discussion already a long time ago on the mailing list, namely that baloo has (had?) zero error handling and simply ass

D19283: try to improve painting height for text lines - bug 403868 avoid to cut _ and other parts still broken: double height things like mixed english/arab, see bug 404713

dhaumann added a comment. That is a good observation: @cullmann Could you give this partial revert a try? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19283 To: cullmann, dhaumann Cc: rjvbb, loh.tar, thomassc, kwrite-devel, kde-frameworks-devel, domson, micha

D19438: Add syntax definition for Elm language to syntax-highlighting

dhaumann added a comment. Could you elaborate what Elm is used for? Also, could you please provide a small test file (also MIT licensed) that we can use for unit testing? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19438 To: bonghyunkim Cc: dhauman

D19407: ModeBase::goToPos: Ensure jump target is valid

dhaumann added a comment. It's FIXED-IN: 5.56... Hm, who is passing an invalid range here? Would it be the same to check r.isValid()? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19407 To: loh.tar, #ktexteditor Cc: dhaumann, kwrite-devel, kde-frameworks-d

D19453: Fix repository deletion warning

dhaumann accepted this revision. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D19453 To: vkrause, cullmann, dhaumann Cc: kossebau, cullmann, kwrite-devel, kde-frameworks-devel, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

D19491: generic config interface

dhaumann added a comment. Currently, one cannot 'unset' a value, but I guess that is ok, since we did not have that before In general this approach is OK. Some thoughts on this: - is it as fast as before, if you start Kate with e.g. 100 documents open? - will this allow easy access

D19532: Don't crash when no contexts are present

dhaumann added a comment. Will this fix https://bugs.kde.org/show_bug.cgi?id=401480 ? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19532 To: davispuh, cullmann, dhaumann, vandenoever, vkrause Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, mich

D19532: Don't crash when no contexts are present

dhaumann added a comment. Could you change the unit test please to not mix with the other ones? - add a new void testBrokenDefinition() - use a local Repository repo; - add custom search oath to this repo - and then check only the broken file. This would be much cleaner and bett

D19532: Don't crash when no contexts are present

dhaumann added a comment. As you wish. Questions remains: does it fix https://bugs.kde.org/show_bug.cgi?id=401480 REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19532 To: davispuh, cullmann, dhaumann, vandenoever, vkrause Cc: kwrite-devel, kde-frame

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann added a comment. 1. Please add a unit test (easy in this case) 2. Please test with right to left text layout, and mixed text layout. 3. Please keep the blinking cursor, this would be separate patch anyways. 4. What happens if you have "persistent selection" enabled? REPOSITORY

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann added a comment. @brauch I think you misunderstood: Select a text in any Qt application (line edit, ...), and then see what happens if you or without holding shift down. This patch is a good one, we just need to sort out possible regressions, and that's it. REPOSITORY R39 KText

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann added a comment. Looks good, now just the autotest is missing :-) REVISION DETAIL https://phabricator.kde.org/D19621 To: loh.tar, #ktexteditor Cc: brauch, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, domson, michaelh, bruns, demsking, cullmann, sars

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann edited the summary of this revision. REVISION DETAIL https://phabricator.kde.org/D19621 To: loh.tar, #ktexteditor Cc: cullmann, brauch, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, domson, michaelh, bruns, demsking, sars

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann added a comment. The unit test is good, but it does not yet test the right-to-left case (e.g. arabic text). Could you add one for this as well? Maybe you can even use KateViewTest::testDeselectByArrowKeys_data() along with QFETCH to reuse the same code in the test function (see

D19517: DocumentPrivate: Add option "Enable Auto Reload" to ModOnHdPrompt

dhaumann added inline comments. INLINE COMMENTS > katedocument.h:1086 > bool m_modOnHd = false; > +bool m_autoReloadMode = false; > +QTimer m_autoReloadThrottle; What about this: make this a QAction with setCheckable(true). Then, you could reuse this action in the KTextEditor::Mess

D19621: ViewPrivate: Make deselection by arrow keys more handy

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Ok with me. I still think that the unit test can be improved, like mixed rtl + ltr text, but ok :) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D1962

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

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

D19517: DocumentPrivate: Add option "Enable Auto Reload" to ModOnHdPrompt

dhaumann added a comment. Just to be on the safe side: Does this also work, if you have two views visible showing the same document, and then the message appears? Does it also work, if you have two main windows showing the same document (View > New Window)? INLINE COMMENTS > kateview.cpp:2

D19681: IconBorder: Accept drag&drop events

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks ok to me. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19681 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel,

D19408: [RFC] Avoid deadlock in use of vi mode searchbar

dhaumann added subscribers: cullmann, dhaumann. dhaumann added a comment. Hm, I currently cannot say anything to this patch. I wonder where the \r comes from. And also, I wonder whether this if now is ever true? ...but if it fixes the issue... @cullmann: objections to a blind commit? REPOSI

D17857: DocumentPrivate: Don't jump view when edit using 'scroll past last line'

dhaumann added a comment. This needs a more detailed review, and currently I find only very few time :( in general, I once tried to fix this bug, but couldn't find a fix. Now you found one - but given this is a very central place I think this needs to be thought through complete prior to com

D19795: Add Variable interface to KTextEditor::Editor

dhaumann created this revision. dhaumann added a reviewer: cullmann. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. dhaumann requested review of this revision. REVISION SUMMARY The Variable interface allows to register either exact matches

D19795: Add Variable interface to KTextEditor::Editor

dhaumann added a comment. What I wonder is whether we really need the "Variable" class. Maybe it would be better to just have - registerExactMatch(QString name, QString description, function) - registerPrefixMatch(QString prefix, QString description, function) If such a change woul

D19798: Fix Words & Chars String on katestatusbar.

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor BRANCH fixStatusBarString (branched from master) REVISION DETAIL https://phabricator.kde.org/D19798 To: ppeter, #kate, dhaumann Cc: pino, kwrite-devel, kde-frameworks-devel, gennad

D19491: generic config interface

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. I see two possible performance issues: 1. Using std::map. ...but the size() is small. 2. See comment. But that needs profiling later. Please push.

D19795: Add Variable interface to KTextEditor::Editor

dhaumann updated this revision to Diff 54069. dhaumann added a comment. - Hide class Variable REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19795?vs=53975&id=54069 BRANCH variable-interface REVISION DETAIL https://phabricator.kde.org/D19795 AFFEC

D19795: Add Variable interface to KTextEditor::Editor

dhaumann added a comment. Unfortunately, I still see one issue: In the unit test, expanding the variable to "World" works, but expanding it to "Smart World" leads to single-quoted text "'Smart World'" because of the space. That of course makes sense, but is not always what you want. The

D19795: Add Variable interface to KTextEditor::Editor

dhaumann updated this revision to Diff 54078. dhaumann added a comment. - Support optional quoting, default is Never REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19795?vs=54069&id=54078 BRANCH variable-interface REVISION DETAIL https://phabricato

D19491: generic config interface

dhaumann added a comment. Agreed, do you plan to migrate more config options now? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19491 To: cullmann, #ktexteditor, dhaumann, loh.tar Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns,

D19795: Add Variable interface to KTextEditor::Editor

dhaumann added a comment. In D19795#432612 , @cullmann wrote: > Hmm. the Variable class and files is just an artifact now or? Yes, it's just used internally. Without it, I'd need to add ugly std::tuple into the QHash. It's still in the KT

D19795: Add Variable interface to KTextEditor::Editor

dhaumann updated this revision to Diff 54089. dhaumann added a comment. - Enum: Use NoQuoting and ShellQuoting REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19795?vs=54078&id=54089 BRANCH variable-interface REVISION DETAIL https://phabricator.kde.

D19795: Add Variable interface to KTextEditor::Editor

dhaumann planned changes to this revision. dhaumann added a comment. Will do another change: since recursive expansion of macros is not supported by KMacroExpander (e.g. %{JS:if(true) %{Document:Selection}}), I'll provide an implementation that does not use KMacroExpander at all. Have that wo

D19795: Add Variable interface to KTextEditor::Editor

dhaumann updated this revision to Diff 54264. dhaumann added a comment. This revision is now accepted and ready to land. - Drop KWordMacroExpander in favor of own simple implementation (50 slocs) REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19795?vs=54

D19795: Add Variable interface to KTextEditor::Editor

This revision was automatically updated to reflect the committed changes. Closed by commit R39:a4083c7cf0d6: Add Variable interface to KTextEditor::Editor (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19795?vs=54264&id=54265 REVISI

D19795: Add Variable interface to KTextEditor::Editor

dhaumann added a comment. It's committed now: - quoting is now not supported at all anymore - expandText() now returns void REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19795 To: dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, gennad, domson,

D19974: Add extensions to groovy highlighting

dhaumann added a comment. You also have to increase the "version" from 5 to 6. Could you provide an updated patch? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19974 To: apushnov, #framework_syntax_highlighting Cc: dhaumann, kwrite-devel, kde-framewor

D19999: Fix endRegion folding in rules with beginRegion+endRegion (use length=0)

dhaumann added a comment. Looks good to me. Anyone else? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D1 To: nibags, #framework_syntax_highlighting, cullmann, dhaumann, vkrause Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraha

D19974: Add extensions to groovy highlighting

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D19974 To: apushnov, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, gennad, bmortimer,

D19876: Fix: apply correctly the text colors of the chosen scheme

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 matte

Re: CI system maintainability

Kevin Ottens schrieb am Do., 28. März 2019, 09:29: > Hello, > > On Thursday, 28 March 2019 09:16:11 CET Ben Cooksley wrote: > > Please note that the commits in this instance were pushed without > > review, so restrictions on merge requests wouldn't make a difference > > in this case unfortunately

D19764: Fix Minimap with QtCurve style

dhaumann added a comment. Thanks for the fix. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19764 To: sars, brauch, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D20264: Add test for "Auto Reload Document" option

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. There is a getter bool KateGlobal::self()->unitTestMode(). You could use this in DocumentPrivate to reduce the timer from 3000 to e.g. 50ms or so to reduce testing time. But beside

D19438: Add syntax definition for Elm language to syntax-highlighting

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. It's also ok to generate the reference data om first commit. Often highlighting contributors do not have a compiling KSyntaxHighlighting git around, so this is a significant hurdle. I'd

D20425: KateIconBorder: Code cosmetic

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D20425 To: loh.tar, cullmann, dhaumann Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, szutmael, gennad, domson, michaelh, ngraham,

<    1   2   3   4   5   6   7   8   9   10   >