D14122: KFormat: Replace unicode literal with unicode codepoint to fix MSVC build

dhaumann added inline comments. INLINE COMMENTS > kformatprivate.cpp:121 > { KFormat::UnitPrefix::Nano, 1e-9, bpow(-30), u'n' }, > -{ KFormat::UnitPrefix::Micro, 1e-6, bpow(-20), u'µ' }, > +// Thanks to broken MSVC, we can not use u'µ', but have to use the > unicode

D14137: CSS: update syntax and fix some errors

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks like a nice update - thanks for your work. Can you 'arc land' yourself? :-) REPOSITORY R216 Syntax Highlighting BRANCH css REVISION DETAIL

D13829: Lua: fix multi-line string

This revision was automatically updated to reflect the committed changes. Closed by commit R216:7d435174e96d: Lua: fix multi-line string (authored by jpoelen, committed by dhaumann). Restricted Application added a project: Kate. Restricted Application added a subscriber: kwrite-devel. REPOSITORY

D14020: RPM Spec: add MIME type

This revision was automatically updated to reflect the committed changes. Closed by commit R216:79df3675d4fc: RPM Spec: add MIME type (authored by nibags, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14020?vs=37491=37738

D14062: Python: fix escapes in quoted-comments

This revision was automatically updated to reflect the committed changes. Closed by commit R216:2627d3e421f6: Python: fix escapes in quoted-comments (authored by nibags, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D14062: Python: fix escapes in quoted-comments

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Thanks! Looks good to me. REPOSITORY R216 Syntax Highlighting BRANCH fix-python REVISION DETAIL https://phabricator.kde.org/D14062 To: nibags, #framework_syntax_highlighting,

D14062: Python: fix escapes in quoted-comments

dhaumann added a comment. Can you add a test line to autotests/input/test.py ? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D14062 To: nibags, #framework_syntax_highlighting, #kate, dhaumann, cullmann Cc: ngraham, kwrite-devel, kde-frameworks-devel,

D13571: Correct KFormat::formatBytes examples

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I think this is an improvement. Even if it can be improved even more, I believe it is better to push this instead of postponing a commit. REPOSITORY R244 KCoreAddons BRANCH

D13970: Handle empty preview lists

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. In any case, this patch will not break things. REPOSITORY R304 KNewStuff BRANCH bugfix/allowEmptyPreviewLists REVISION DETAIL https://phabricator.kde.org/D13970 To:

D13940: Add syntax highlighting support for Stan

dhaumann added a comment. Looks already pretty good to me, but indeed highlight.stan is missing - the test file for the autotest. Could you add it and update the patch? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D13940 To: jeffreyarnold,

D13380: haskell.xml: don't highlight Prelude data constructors differently from others

This revision was automatically updated to reflect the committed changes. Closed by commit R216:44bc7c609dad: haskell.xml: dont highlight Prelude data constructors differently from others (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13380?vs=35703=37385#toc

D13380: haskell.xml: don't highlight Prelude data constructors differently from others

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH haskell-data-prelude REVISION DETAIL https://phabricator.kde.org/D13380 To: xialiyao, dhaumann Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13386: haskell.xml: remove types from "prelude function" section

This revision was automatically updated to reflect the committed changes. Closed by commit R216:c78ba1bd98dc: haskell.xml: remove types from prelude function section (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D13386: haskell.xml: remove types from "prelude function" section

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Nice patches, thanks! REPOSITORY R216 Syntax Highlighting BRANCH haskell-remove-nonsense REVISION DETAIL https://phabricator.kde.org/D13386 To: xialiyao,

D13373: haskell.xml: highlight promoted data constructors

This revision was automatically updated to reflect the committed changes. Closed by commit R216:6db3d08c8ecb: haskell.xml: highlight promoted data constructors (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D13373: haskell.xml: highlight promoted data constructors

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH haskell-promoted-types REVISION DETAIL https://phabricator.kde.org/D13373 To: xialiyao, #framework_syntax_highlighting, dhaumann Cc: kde-frameworks-devel,

D13370: haskell.xml: add keywords family, forall, pattern

This revision was automatically updated to reflect the committed changes. Closed by commit R216:7a45d5e122ad: haskell.xml: add keywords family, forall, pattern (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13370?vs=35649=37382#toc REPOSITORY R216 Syntax

D13370: haskell.xml: add keywords family, forall, pattern

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Can you for future review requests please add some description of what the patch changes? That would be very helpful for reviewing. REPOSITORY R216 Syntax Highlighting BRANCH

D13970: Handle empty preview lists

dhaumann added a comment. Which bug in which context does this fix? The information given is a bit 'thin' ;-) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13970 To: cordlandwehr, gregormi, nicolasfella, dhaumann, #frameworks Cc: kde-frameworks-devel, michaelh,

D13635: Fixes for Java comments

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I'll give the OK, but please increase kateversion to 5.0 first. INLINE COMMENTS > javadoc.xml:3 > > - extensions="" license="LGPL" author="Alfredo Luiz Foltran Fialho >

D13884: [KMessageWidget] Update stylesheet when palette changes

dhaumann added a comment. Late to the game, but also +1 from me. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D13884 To: broulik, #frameworks, rjvbb, ngraham, cfeck Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D13829: Lua: fix multi-line string

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. REPOSITORY R216 Syntax Highlighting BRANCH lua REVISION DETAIL https://phabricator.kde.org/D13829 To: jpoelen, dhaumann, cullmann, #ktexteditor, #kate Cc:

D13880: [KMoreTools] Reduce menu hierarchy

dhaumann added a comment. I like the idea as well. +1 REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13880 To: nicolasfella, gregormi, dhaumann, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13706: [KMoreTools] Enable installing tools via appstream url

dhaumann added a comment. The API documentation can / should still be improved. I tried to give an example of possible improvements. INLINE COMMENTS > kmoretools.h:490 > + * @since 5.48 > + * @return The service' appstream id > + */ What about this: /** * Returns the

D13706: [KMoreTools] Enable installing tools via appstream url

dhaumann resigned from this revision. dhaumann added a comment. This revision is now accepted and ready to land. @ngraham Since you know more about installation (e.g. via Discover), I would like you to give another +1, if possible. REPOSITORY R304 KNewStuff BRANCH appstream REVISION

D13706: [KMoreTools] Enable installing tools via appstream url

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Looks already pretty good to me, but please add API documentation. Rule of thumb: if you extend a header file that is documented, then follow this scheme and also

D13706: [KMoreTools] Enable installing tools via appstream url

dhaumann added a reviewer: gregormi. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D13706 To: nicolasfella, #frameworks, gregormi Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13394: C++: update for c++20 and fix some syntax errors

dhaumann added a comment. @jpoelen Can you push yourself, or do you still not have a KDE contributor account? REPOSITORY R216 Syntax Highlighting BRANCH cpp REVISION DETAIL https://phabricator.kde.org/D13394 To: jpoelen, dhaumann, cullmann Cc: cullmann, kde-frameworks-devel,

D13657: Highlight Gradle files with Groovy syntax too

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D13657 To: vkrause, dhaumann Cc: dhaumann, kde-frameworks-devel,

D13026: Make menu-bearing toolbar buttons show their menus with normal click rather than click-and-hold

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Lgtm - thanks! Please only in master (as this review request shows). REPOSITORY R39 KTextEditor BRANCH schema-toolbar-button-opens-on-normal-click (branched from master) REVISION

D13541: Port solid from Qt5::Widgets to Qt5::Gui

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. const auto would be even nicer, if possible. Besides that, lgtm. REPOSITORY R245 Solid BRANCH gui-instead-of-widgets REVISION DETAIL https://phabricator.kde.org/D13541 To:

D13442: Implemented displaying of total lines in kate

dhaumann added a comment. +1 for the context menu on the line/column fields. By the way, the argument that " of " is short does not hold, since translations into other languages may result in much longer text. REPOSITORY R39 KTextEditor REVISION DETAIL

D13394: C++: update for c++20 and fix some syntax errors

dhaumann added a subscriber: cullmann. dhaumann added a comment. To be honest, I cannot claim I can follow all changes, since this is a big update. But in general the usage of the rules look good. The autotest output (html highlighting) looks good to me, it contains nice improvements

D13498: Rust: Add keywords & bytes, fix identifiers, and other improvements/fixes

This revision was automatically updated to reflect the committed changes. Closed by commit R216:601f93780573: Rust: Add keywords bytes, fix identifiers, and other improvements/fixes (authored by nibags, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D13498: Rust: Add keywords & bytes, fix identifiers, and other improvements/fixes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Nice update, will integrate. And thanks for the unit test! REPOSITORY R216 Syntax Highlighting BRANCH improve-rust (branched from master) REVISION DETAIL

D13442: Implemented displaying of total lines in kate

dhaumann added a comment. @cullmann Hm, ist showing the maximum column really important. You'd need to compute this over the entire document ... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor, cullmann, brauch Cc: ngraham,

D13442: Implemented displaying of total lines in kate

dhaumann added a comment. Personally, I belong to the group that does not care about the total number of lines. To me, this is visual noise. The current line number is relevant for me, since it identifies a unique location in the document. But if you think this is required I will not stop

D12768: Allow wrapping selection off top/bottom of autocomplete results

dhaumann added a comment. Indeed, question is if wrapping helps: If you see 5 entries, and you see the last one is correct, then moving up would be ok. But if the completion contains >> 10 entries, this will not work. And then, there is still the "End" key on the keyboard as dedicated

D12271: Don't remove trailing whitespace from cursor line

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. See arguments above: If we change it, another one will come over

D12768: Allow wrapping selection off top/bottom of autocomplete results

dhaumann added subscribers: cullmann, dhaumann. dhaumann added a comment. I remember that @cullmann intentionally implemented it this way: Going upwards will hide the completion list and not wrap around. @cullmann Any take on this? Either accept, or reject. Also, the unit test need

D12854: Awk: fix regex in a function and update syntax for gawk

This revision was automatically updated to reflect the committed changes. Closed by commit R216:d5c5f4d3c85a: Awk: fix regex in a function and update syntax for gawk (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D12854: Awk: fix regex in a function and update syntax for gawk

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me, will integrate. REPOSITORY R216 Syntax Highlighting BRANCH awk REVISION DETAIL https://phabricator.kde.org/D12854 To: jpoelen, #framework_syntax_highlighting,

D12897: Reserve space for the cachedLineForRanges Qhash (optimization)

dhaumann added subscribers: mwolff, dhaumann. dhaumann added a comment. @mwolff To me this looks ok - do you see an issue with this? E.g. that KTextEditor will require much more memory for almost no gain? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12897 To:

D13026: Open Schema toolbar button with normal click rather than click-and-hold

dhaumann added a comment. The same holds true for other actions such as: Tools > Highlighting, Mode, Indentation, Script. Question here is: Is this delayed thing needed for building the contents on-the-fly (look at the aboutToShow() functions)? REPOSITORY R39 KTextEditor REVISION

D13395: Pony: fix single quote escape and a possible infinite loop with #

This revision was automatically updated to reflect the committed changes. Closed by commit R216:0ec0100e5948: Pony: fix single quote escape and a possible infinite loop with # (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D13395: Pony: fix single quote escape and a possible infinite loop with #

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Works - will integrate. Thanks! REPOSITORY R216 Syntax Highlighting BRANCH fix-pony REVISION DETAIL https://phabricator.kde.org/D13395 To: jpoelen, dhaumann Cc:

D12233: Avoid manipulation of lists with quadratic complexity

dhaumann added a comment. I would give a ship-it - but maybe another review would be good? @ngraham maybe? INLINE COMMENTS > bruns wrote in pendingfilequeue.cpp:69 > GCC generates identical code for both ... Did you check? This is certainly correct for i++ if i is an int. But for

D12860: DoxygenLua: fix closing comment blocks

This revision was automatically updated to reflect the committed changes. Closed by commit R216:b136c9fd1c49: DoxygenLua: fix closing comment blocks (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12860?vs=34096=35147

D12860: DoxygenLua: fix closing comment blocks

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Will integrate, thanks. REPOSITORY R216 Syntax Highlighting BRANCH luafix REVISION DETAIL https://phabricator.kde.org/D12860 To: jpoelen, #framework_syntax_highlighting,

D12689: Lua: updated for Lua5.3

dhaumann added a subscriber: cullmann. dhaumann added a comment. @vkrause @cullmann Could you please comment on the above? Should we change KTextEditor such that dynamic is automatically derived based on whether there is a dynamic rule? Indeed, I agree with @jpoelen that this is redundant.

D13067: add pgf to the latex-ish file formats (same format as tikz)

dhaumann added a comment. @jan.hajer Thanks for the patch, updates like this are very much appreciated, so please keep it coming :-) The change will be in KDE Frameworks 5.47, released early in June. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D13067

D13067: add pgf to the latex-ish file formats (same format as tikz)

This revision was automatically updated to reflect the committed changes. Closed by commit R216:770d1515db89: add pgf to the latex-ish file formats (same format as tikz) (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D13067?vs=34727=35145#toc REPOSITORY R216

D12047: Avoid crash when reading corrupt data from document terms db

dhaumann added inline comments. INLINE COMMENTS > documentdb.cpp:101 > +if (result.isEmpty()) { > +qDebug() << "Document Terms DB contains corrupt data for " << docId; > +} Ah, maybe this should be a qWarning()? Feel free to decide as you wish. REPOSITORY R293 Baloo BRANCH

D12047: Avoid crash when reading corrupt data from document terms db

dhaumann added a comment. Ok, then please commit. REPOSITORY R293 Baloo BRANCH b392878_avoid_docterms_decode_crash REVISION DETAIL https://phabricator.kde.org/D12047 To: bruns, #baloo, michaelh, ngraham, #frameworks, dhaumann Cc: dhaumann, kde-frameworks-devel, #frameworks,

D12025: Terminate query execution early if subterm returns empty result set

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH terminate_empty_terms_early REVISION DETAIL https://phabricator.kde.org/D12025 To: bruns, #baloo, michaelh, #frameworks, dhaumann Cc: kde-frameworks-devel, #frameworks,

D13067: add pgf to the latex-ish file formats (same format as tikz)

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. This patch is correct, except that the version needs to be increased. Will integrate later. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL

D13067: add pgf to the latex-ish file formats (same format as tikz)

dhaumann added a project: Framework: Syntax Highlighting. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D13067 To: jan.hajer Cc: kde-frameworks-devel, michaelh, genethomas, ngraham, bruns, cullmann, vkrause, dhaumann

D12233: Avoid manipulation of lists with quadratic complexity

dhaumann added a comment. Looks good to me. INLINE COMMENTS > pendingfilequeue.cpp:69 > +const auto droppedFilesBegin = std::partition(m_cache.begin(), end, > keepFile); > +for (auto it = droppedFilesBegin; it != end; it++) { > +m_pendingFiles.remove(it->path());

D12047: Avoid crash when reading corrupt data from document terms db

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. If the format is really such that a term must appear before any Suffix, then this patch is already better that before. Could it happen to have e.g.: a\0b1c1 If so, this code

D11685: Implement single click on line number to select line of text

dhaumann added a comment. Ok, thanks! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11685 To: rkron, #frameworks, #kate, #ktexteditor, ngraham, cullmann Cc: kwrite-devel, kde-frameworks-devel, dhaumann, rkron, mwolff, ngraham, #ktexteditor, #kate, #frameworks,

D11685: Implement single click on line number to select line of text

dhaumann added a comment. Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. Btw, did anyone test with dynamic word wrap enabled? Does it still work, when a line spans multiple view lines? REPOSITORY R39 KTextEditor REVISION DETAIL

D7245: Improve reStructuredText highlighting

dhaumann added a comment. @turbov ping again :-) After adding the reference file in autotest/input, run make test, and then call ./autotests/update-reference-data.sh in your build folder. Then git status will tell you which files are new and need to be added updated. REVISION DETAIL

D10719: Highlighting for OpenSCAD

This revision was automatically updated to reflect the committed changes. Closed by commit R216:849aea294446: Highlighting for OpenSCAD (authored by dhaumann). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10719?vs=28190=34985#toc REPOSITORY R216 Syntax Highlighting CHANGES SINCE

D10719: Highlighting for OpenSCAD

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. Looks good, will integrate! REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D12689: Lua: updated for Lua5.3

dhaumann added a subscriber: vkrause. dhaumann added a comment. @jpoelen And why is it a problem with Kate? This means the C++ implementation of the highlighting rules is different in KSyntaxHighlighting and KTextEditor? If so, this would be an important catch. Can you elaborate why this is

D12689: Lua: updated for Lua5.3

dhaumann added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. @jpoelen Could you have a look at https://bugs.kde.org/show_bug.cgi?id=394184 ? It seems we have a regression? REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D12831: Don't automatically set the default icons for each style

dhaumann added a comment. Thanks! REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D12831 To: ngraham, cullmann, cfeck Cc: dhaumann, cfeck, kde-frameworks-devel, michaelh, ngraham, bruns

D9219: WIP: Extend Scripting API

dhaumann added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. @cullmann ping? :-) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9219 To: dhaumann, cullmann, mwolff Cc: kwrite-devel,

D12508: Make KMessageWidget match Kirigami inlineMessage's style

dhaumann added a comment. I think the setIcon() is also buggy, since if you call setIcon() and then setMessageType(), the icon is lost. So yes, I would be in favor of a better solution. If you revert the setIcon() part, do the screenshot examples all loose their icon? Or is it still

D12508: Make KMessageWidget match Kirigami inlineMessage's style

dhaumann added a comment. Somehow my other comments were lost, here we go: - could you also update the screenshot in the doxygen documentation? - setIcon() is behavior incompatible, and in fact, the referenced bugs did not complain about icons. So why the change? In my opinion this is

D12508: Make KMessageWidget match Kirigami inlineMessage's style

dhaumann added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. Can we revert the setIcon() part? It changes application behavior, an in the case of Kate/KWrite, this is note wanted, see my comments. REPOSITORY R236 KWidgetsAddons

D12569: Pony: fix identifier and keyword

This revision was automatically updated to reflect the committed changes. Closed by commit R216:ec6486460a6a: Pony: fix identifier and keyword (authored by jpoelen2, committed by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D12569: Pony: fix identifier and keyword

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good, will integrate. REPOSITORY R216 Syntax Highlighting BRANCH fix_pony REVISION DETAIL https://phabricator.kde.org/D12569 To: jpoelen, dhaumann Cc: #frameworks,

D12689: Lua: updated for Lua5.3

dhaumann closed this revision. dhaumann added a comment. Committed, see https://commits.kde.org/syntax-highlighting/9f11b4fa2604c02e9a15f1949a8455283e9c0d62 REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D12689 To: jpoelen, dhaumann,

D12662: Add InlineNoteInterface

dhaumann added inline comments. INLINE COMMENTS > dhaumann wrote in katedocument.cpp:5316-5320 > This look like premature optimization. I would prefer to delete this "common > cases first" block. Except if this turned out to be a bottleneck already? Btw, may I am wrong here, since this is

D12662: Add InlineNoteInterface

dhaumann added a comment. This is already an excellent patch. The API documentation is already really nice. I have some minor suggestions (see inline comments in patch). Besides that, I wonder whether this should really be a View extension interaface. Currently, this interface is

D12587: Indentation script for R

dhaumann added a comment. @devillemereuil If I remember correctly, you do not have to run all tests. It is enough to start the indenter test (look into the bin folder) and add as parameter you indent test folder (relative, e.g. cstyle). And, what Thomas says is correct. Typically a test

D12587: Indentation script for R

dhaumann added a comment. I think this is very cool. But there is one thing we require for new indenters: unit tests. There is a subfolder autotest in the KTextEditor git module. Could you have a look into this? Without such automated tests, we will likely introduce regressions in

D12271: Don't remove trailing whitespace from cursor line

dhaumann added a comment. @sraizada I am just seeing that this is your first KDE patch, nice! I hope that you do not get discouraged by my comments, I am just trying to find good solutions that work for everyone, and in this case, the solution this patch proposed did not work out in the

D12271: Don't remove trailing whitespace from cursor line

dhaumann added a comment. The behavior that is proposed here was like Kate behaved before, and as result we got bug reports that not all trailing spaces were removed. So we removed this. I can see that when saving very often, this may be annoying. On the other hand, the current

D11945: Optimize AppArmor & SELinux highlighting and improve regex

dhaumann added a comment. Out of curiosity: why are you putting multiple items in the same line on the keyword lists? This bloats up the diff and makes a real review so much harder... :/ REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11945 To: nibags,

D7175: Redesign CMake syntax

dhaumann added a comment. Sure? It was reverted a month ago once. Was it applied again? REVISION DETAIL https://phabricator.kde.org/D7175 To: turbov, dhaumann, #kate, #framework_syntax_highlighting, vkrause, cullmann Cc: cullmann, #frameworks, michaelh, ngraham, bruns

D12221: Fix problem that font/italic/... attributes no longer work with e.g. >= Qt 5.9

dhaumann added a comment. @ngraham So what is the current state of a proper fix in KFontRequester? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12221 To: cullmann, #frameworks, #kate, dhaumann Cc: ngraham, dhaumann, michaelh, kevinapavew, bruns, demsking,

D12221: Fix problem that font/italic/... attributes no longer work with e.g. >= Qt 5.9

dhaumann added a comment. See: - https://bugs.kde.org/show_bug.cgi?id=376094 - https://bugs.kde.org/show_bug.cgi?id=383665 - https://bugs.kde.org/show_bug.cgi?id=387401 - https://bugs.kde.org/show_bug.cgi?id=391040 They all are closed as a general KFontChooser issue. Maybe we

D12221: Fix problem that font/italic/... attributes no longer work with e.g. >= Qt 5.9

dhaumann added a comment. Btw there are bug reports for this. Could you link the Kate related ones, if existing? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12221 To: cullmann, #frameworks, #kate, dhaumann Cc: dhaumann, michaelh, kevinapavew, ngraham, bruns,

D12221: Fix problem that font/italic/... attributes no longer work with e.g. >= Qt 5.9

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. A simple fix for a severe issue - please commit! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12221 To: cullmann, #frameworks, #kate, dhaumann Cc:

D12215: Add a "Reload" menu item to KDirOperator's context menu

dhaumann added a comment. I am ok with this: +1 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12215 To: ngraham, #frameworks Cc: dhaumann, rkflx, michaelh, ngraham, bruns

D11470: SQL: various improvements and fix if/case/loop/end detection with SQL (Oracle)

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Looks mostly good, but there are some minor issues we first need to address: - dsControlFlow and similar default styles were added with KDE Frameworks 5, so

D11838: Turn on line numbers by default

dhaumann added a comment. As background: in KF5 world, the KTextEditor settings are shared among applications: enabling line numbers in Kate will enable line numbers in KDevelop, Kile, KWrite, ... Currently, there is no way to show line numbers except in Kile. I can understand that

D11838: Turn on line numbers by default

dhaumann added a subscriber: kfunk. dhaumann added a comment. This looks good from my side. @kfunk is this ok also for KDevelop? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11838 To: ngraham, #kate, #ktexteditor, dhaumann Cc: kfunk, dhaumann,

D11838: Turn on line numbers by default

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Not yet good enough, let's have another revision. BTW, I am fine with this change, since nowadays screens are typically wider, so horizontal space is usually there.

D11610: clang-tidy: modernize-use-default-member-init run

dhaumann added a comment. Btw, I messed up the dates. Tag is next Saturday after today... REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D11610 To: kfunk, dhaumann Cc: dhaumann, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann,

D11610: clang-tidy: modernize-use-default-member-init run

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. The patch looks good. Please commit this weekend, or wait until next Saturday's v5.45 branch. REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL

D11610: clang-tidy: modernize-use-default-member-init run

dhaumann added a comment. I like this a lot, but have not done a review yet. Will do later. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11610 To: kfunk Cc: dhaumann, #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars

D11543: Optimize many syntax highlighting files and fix the '/' char of SQL

dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed. Cool! The patch already looks pretty good. Please address or comment on my questions. And for a final round, I would like to have a review by another reviewer, since

D11491: Don't calculate attribute() twice.

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11491 To: jtamate, #frameworks, #kate, dhaumann Cc: dhaumann, michaelh, kevinapavew,

D11298: Optimize highlighting Bash, Cisco, Clipper, Coffee, Gap, Haml, Haskell

This revision was automatically updated to reflect the committed changes. Closed by commit R216:cec763566ca4: Optimize highlighting Bash, Cisco, Clipper, Coffee, Gap, Haml, Haskell (authored by dhaumann). REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE

D10621: Highlighting Indexer: list of suggestions

dhaumann added a comment. @jpoelen What would be interesting is to check which optimizations are really an improvement. Because we should either get a significant speed boost (e.g. RegExpr -> WordDetect), or at least reduce memory allocations (possibly StringDetect -> Detect2Chars and

D11298: Optimize highlighting Bash, Cisco, Clipper, Coffee, Gap, Haml, Haskell

dhaumann created this revision. dhaumann added reviewers: vkrause, jpoelen. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. dhaumann requested review of this revision. REVISION SUMMARY These are some of the optimizations of D10621

D10621: Highlighting Indexer: list of suggestions

dhaumann added a comment. Just for info: https://kate-editor.org/2018/03/10/improving-syntax-highlighting-files/ tries to reach a broader audience for getting patches. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D10621 To: jpoelen, dhaumann Cc:

<    3   4   5   6   7   8   9   10   11   12   >