Re: KF6 meeting notes 2021-04-24

Hi Luigi, with respect to the reminder of the weekly meetings: It might make sense to send such a reminder *before* these meetings and not after :-) Besides that, thanks for sending these minutes to keep us all informed. Best regards Dominik On Sat, Apr 24, 2021 at 4:19 PM Luigi Toscano

Re: KDE review for KWeatherCore

Hi Han, let's have this discussion public on kde-core-devel and kde-framworks, so other can contribute. On Sat, Jan 2, 2021 at 1:29 PM hanyoung wrote: > > Hi Dominik, > > Thank you for your time. This is my first time writing a library, I tried my > best by looking > at existing KDE frameworks

Re: KDE review for KWeatherCore

Hi Han, just having a quick glance at the code, and I feel like there is a lot of polishing missing. I'll give some examples, but this is by no means a complete review, and I'll not comment on technical aspects at all, since this is not the domain of my expertise. - #pragma once: It's used in

D29789: Make text always align with font base line

dhaumann added a comment. I like this patch. In fact, I observed over the past years again and again that sometimes, especially if chinese or similar letters are included, the baseline is wrong in Kate, leading to much more overpainting that needed. If this patch fixes this, then I'm all

D27844: Store and fetch complete view config in and from session config

dhaumann added a comment. I suggest to revert, and send a notification with the change to kde-distro-packa...@kde.org to avoid that many users break their configuration. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D27844 To: kossebau, #kate, loh.tar,

D29735: Assign "Identifier" attribute to opening double quote instead of "Comment"

dhaumann added a subscriber: cullmann. dhaumann added a comment. @cullmann could you integrate this? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D29735 To: bewuethr, #framework_syntax_highlighting, dhaumann Cc: cullmann, dhaumann, kwrite-devel,

D29735: Assign "Identifier" attribute to opening double quote instead of "Comment"

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Good catch :) please commit REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D29735 To: bewuethr, #framework_syntax_highlighting, dhaumann Cc:

D25339: update lineHeight if boundingRect indicates a larger value.

dhaumann added a comment. In D25339#666832 , @cullmann wrote: > Looking at the code, might it make more sense to just move away from the fixed height we have? Yes please. Instead of hacks and options, which will make the code and the UI

D28039: optimize dynamic regex matching

dhaumann added a comment. Thanks yes, maybe you can add a comment to the skipOffset, Christoph :) REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D28039 To: cullmann, dhaumann, vkrause, nibags Cc: mwolff, kwrite-devel,

D28039: optimize dynamic regex matching

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I guess this is OK, but the concept of a "skip offset" is a bit fuzzy to me. INLINE COMMENTS > rule_p.h:126 > + > +protected: > +bool m_dynamic = false; I dislike this protected

D28015: extend indexer to detect dynamic=true regexes that have no place holders to adapt

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I guess in a followup commit the reported issues will be fixed? :-) REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D28015 To:

D27928: [VIM Mode] Add g g commands

dhaumann added a comment. The intimidating one :) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D27928 To: gikari, #kate, #frameworks Cc: dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, domson, michaelh, ngraham, bruns, demsking,

D27916: Add Overpass QL highlighting

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Almost good, please fix + ship. INLINE COMMENTS > overpassql.xml:7 > +--> > + extensions="*.overpassql" author="Volker Krause (vkra...@kde.org)" > license="MIT"> > + For

D27929: CMake syntax fixes

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Thanks! REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D27929 To: turbov, #framework_syntax_highlighting, dhaumann Cc: dhaumann, kwrite-devel,

D27928: [VIM Mode] Add g g commands

dhaumann added a comment. I like the patch, but please add a unit test before we commit this. See https://github.com/KDE/ktexteditor/tree/master/autotests/src/vimode Could you add one? :) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D27928 To: gikari, #kate,

D27844: Store and fetch complete view config in and from session config

dhaumann added a comment. I guess we can give this a try. As I understand, though, with this patch you will never be able to use e.g. a global zoom once you change the zoom of a view. This was different before this patch. Which brings me to another point: e.g. zoom should simply zoom

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kateviewhelpers.cpp:1963 > You should keep devicePixelRatioF calls This is true. But in this case a non-answer: Maybe QIcon::paint does it correct as well. In other words, the code can very likely be improved, but also

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor BRANCH addmarkinterfacev2 REVISION DETAIL https://phabricator.kde.org/D27533 To: kossebau, #kate, #kdevelop, dhaumann Cc: anthonyfieroni, dhaumann, kwrite-devel,

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added a comment. Ok, then I am fine with this. Maybe add a KF6 task to the KF6 board? INLINE COMMENTS > kossebau wrote in katedocument.h:86 > See the warning in the API docs, nobody should rely on this private API, so > the BIC here is okay. > The class is exported for the unit

D27533: [WIP] Add MarkInterfaceV2, to s/QPixmap/QIcon/g for symbols of marks

dhaumann added a comment. Definitely a +1 for using a QIcon here. Still, I wonder whether we should postpone adding the MarkInterfaceV2 until the KF6 branch. Then we have it properly fixed in KF6. Or do you think we also need this in KF5 days to properly support e.g. hidpi ?

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

dhaumann added a subscriber: cullmann. dhaumann added a comment. Ok, I now fully got this, thanks for the explanation. And indeed without any left fill visual feedback is missing. What I wonder is how much of this needs to be configurable. We could also just make the left fill of 4

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

dhaumann added a comment. No offense meant, but even with the screenshots I still have no idea what this is about :) Can you add a before/after screenshot so we can see the visual difference? Also, the "fill left" and "fill right" wording is new, and not intuitively understandable

D27238: Add an option to dynamic-break inside words

dhaumann added subscribers: cullmann, dhaumann. dhaumann added a comment. I think this patch looks good. @cullmann: this originates from https://www.reddit.com/r/kde/comments/ey4158/comment/fgoycn4?context=3 Any opinion? REPOSITORY R39 KTextEditor REVISION DETAIL

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Better :) with corners I mean the 1-3 pixels left due to the rounding corners. These pixels were once also drawn as background although they are outside of the frame. It may be a minor detail, but imho such details are important. But indeed, the screenshots look

D26697: Message-related methods: Use more member-function-pointer-based connect

dhaumann added a comment. In two days we have the next KDE Frameworks tag. I'd have preferred a commit on Sunday ;) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26697 To: kossebau, dhaumann, cullmann Cc: cullmann, kwrite-devel, kde-frameworks-devel, rrosch,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Can you provide screenshots of more styles? :-) REPOSITORY R236 KWidgetsAddons BRANCH no-css (branched from master) REVISION DETAIL https://phabricator.kde.org/D27035 To: davidre, #frameworks, ngraham Cc: dhaumann, ngraham, kde-frameworks-devel, LeGast00n,

D27035: [KMessageWidget] Draw it with QPainter instead of using stylesheet

dhaumann added a comment. Does that also work in Kate for floating messages like when the search wraps? What I want to know is whether the corners behind the green frame are transparent in this case, or whether the corners are painted solid. If I remember correctly, these kind of bugs were

D26903: allow programLogo property to be a QIcon, too

dhaumann added a comment. Looks good to me, and very much in line with the other `canConvert` statements before. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D26903 To: cullmann Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26812: Variable expansion: Add variable PercentEncoded

This revision was automatically updated to reflect the committed changes. Closed by commit R39:b0055e8476bb: Variable expansion: Add variable PercentEncoded (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26812?vs=74032=74034

D26811: Fix crash in variable expansion (used by external tools)

This revision was automatically updated to reflect the committed changes. Closed by commit R39:22fa707eb7e2: Fix crash in variable expansion (used by external tools) (authored by dhaumann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE

D26812: Variable expansion: Add variable PercentEncoded

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 This can be used to encode the input text such that it can

D26811: Fix crash in variable expansion (used by external tools)

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 findClosing() is already called with +2 to account for the

D25698: New query mechanism for applications: KApplicationTrader

dhaumann added inline comments. INLINE COMMENTS > kapplicationtrader.cpp:87 > +KService::List::iterator it = lst.begin(); > +while (it != lst.end()) { > +KService::Ptr serv = *it; I would prefer the std::erase(std::remove_if(...), ...end()); idiom here. Assuming the list is a

D26779: Updates nasm.xml with the latest instructions

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me. Can you commit or shall we push this for you? REPOSITORY R216 Syntax Highlighting BRANCH nasm (branched from master) REVISION DETAIL

D26755: [WIP] KMessageWidget: Set widget height on resize event

dhaumann added a comment. Well, it definitely would be very nice if you find a patch that fixes this. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D26755 To: SGOrava, #frameworks Cc: anthonyfieroni, dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D26755: [WIP] KMessageWidget: Set widget height on resize event

dhaumann resigned from this revision. dhaumann added a comment. This revision now requires review to proceed. In that case I withdraw my review for now. I can only speak for Kate that I could not find a regression with my tests. But if you find other regressions, then this is likely not

D26755: KMessageWidget: Set widget height on resize event

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I just tested with KTextEditor's messagetest and KTextEditor's usage of KMessageWidget in the top and bottom bar, and all this looks still good. +1 from my side. Another review

D26755: KMessageWidget: Set widget height on resize event

dhaumann added a comment. Since KTextEditor uses KMessageWidget extensively we have to make sure the unit test messagetest still passes with this change. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D26755 To: SGOrava, #frameworks Cc: dhaumann,

D26704: KateMessageWidget: remove unused event filter installation

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. I guess this change is correct :-) REPOSITORY R39 KTextEditor BRANCH removeunusedeventfilter REVISION DETAIL https://phabricator.kde.org/D26704 To: kossebau, dhaumann Cc:

D26560: Fix selection printing

dhaumann added a comment. That looks already better: If a line is wrapped and on the same page, then only the selected text is printed. It seems there are still corner cases. 1. Create a document with just one long line that wraps over two printed pages. I this case, I am not able to

D26463: Port endl as \n or std::cout ... std::endl when we wrote directly on console or flush QTextStream when it's a qstring

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R39 KTextEditor BRANCH port_endl (branched from master) REVISION DETAIL https://phabricator.kde.org/D26463 To: mlaurent, dfaure, dhaumann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2,

D26424: [kdiroperator] Add method for accessing actions without KActionCollection

dhaumann added inline comments. INLINE COMMENTS > kdiroperator.h:428 > +/** > + * an accessor to a collection of all available Actions. The actions > + * are static, they will be there all the time. Imho the API documentation should be improved (I am aware it was copied). Returns

D26321: Expose show-line-count in the ConfigInterface

dhaumann accepted this revision. dhaumann added a comment. Ok with this... Thanks. A note " Since 5.6x would be nice". REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D26321 To: mludwig, #ktexteditor, cullmann, dhaumann Cc: dhaumann, cullmann, kwrite-devel,

Re: CMake config & target challenges on moving to KF5 namespace; dir structure & API dox (Re: Submitting Grantlee as a KF5 Framework)

Hi, Stephen Kelly schrieb am So., 29. Dez. 2019, 15:03: > > On 28/12/2019 23:30, Friedrich W. H. Kossebau wrote: > > Why are you proposing to do a step back instead to the old state, which > > everyone including you considered not that satisfying? > > Because it's a temporary situation. We

D21563: Fix empty pages in print preview and lines printed twice

dhaumann added a comment. @hoffmannrobert: by the way, looking at your phabricator activity, you should get a KDE contributor account, if you don't have one already. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D21563 To: hoffmannrobert, #kate, #ktexteditor,

D21563: Fix empty pages in print preview and lines printed twice

dhaumann added a comment. @hoffmannrobert: Are you maybe also interested in looking into https://bugs.kde.org/show_bug.cgi?id=415570 ? It's again about printing, this time about the very last line that seems to be wrong... REPOSITORY R39 KTextEditor REVISION DETAIL

Re: Exemptions to try KF "grow" vs. consistent experience (Re: Submitting Grantlee as a KF5 Framework)

57 CET schrieb Volker Krause: > > > > On Sunday, 22 December 2019 09:46:02 CET Dominik Haumann wrote: > > > > > Hi all, > > > > > > > > > > in any case, maybe the discussed points should go to the KF6 > > > > > workboard?

Re: CMake config & target challenges on moving to KF5 namespace; dir structure & API dox (Re: Submitting Grantlee as a KF5 Framework)

Hi all, in any case, maybe the discussed points should go to the KF6 workboard? https://phabricator.kde.org/project/view/310/ I indeed believe that consistency in the KF5 world is an important feature, so Friedrich does have a point here. Other framework additions had to adapt as well (what

D25870: SELinux: add "glblub" keyword and update permissions list

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

D25869: Python: improve numbers, add octals, binaries and "breakpoint" keyword

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

D23457: Port regex search to QRegularExpression

dhaumann added a comment. In D23457#574846 , @ahmadsamir wrote: > Personally, I am not convinced replacing \s with [ \t], and deviating from PCRE default behaviour is a good idea in this case. That is not to be discussed, and Christoph

D23457: Port regex search to QRegularExpression

dhaumann added a comment. Was anything of the previously commented issues addressed? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D23457 To: ahmadsamir, #ktexteditor, dhaumann, cullmann Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh,

D25767: KAutoSaveFile: add a unit test to check max. filename length

dhaumann added inline comments. INLINE COMMENTS > dfaure wrote in kautosavefiletest.cpp:98 > Let's hope none of the supported OSes have a smaller PATH_MAX :) > FreeBSD, Windows, macOS, who knows what limits they have. What I wonder is: PATH_MAX etc are #defines, right? Since on Windows there is

D25755: Replace iterators with range-based for

dhaumann added a comment. >>> Do we dislike iterators now? >> >> We don't, and they still make sense for when you need the `key`, but range for is just much nier to look at :) > > I'm fine with that statement. But are we going to be reviewing changing all the KDE code from

D25770: Several enhancements to gitolite syntax definition.

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good. But does the highlighting work for `RW+CD`? I am wondering whether `+` needs to be added to the weakDeliminator list? REPOSITORY R216 Syntax Highlighting BRANCH

D25276: reStructuredText: Fix inline literals highlighting preceding characters

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Ok REPOSITORY R216 Syntax Highlighting BRANCH rst-inline-literal-prefix REVISION DETAIL https://phabricator.kde.org/D25276 To: adrianchavesfernandez, mosra, dhaumann Cc:

D25698: New class KApplicationTrader, to replace KMimeTypeTrader and KServiceTypeTrader

dhaumann added inline comments. INLINE COMMENTS > kapplicationtrader.h:33 > + * > + * Example: say that you want to the list of all applications that can > handle PNG images. > + * The code would look like: Typo: you want to _get_... REPOSITORY R309 KService REVISION DETAIL

D25493: EBN kmoretools cleanup transport protocol

dhaumann added a comment. @jhayes Do you have commit access, or shall I commit this for you? REPOSITORY R304 KNewStuff BRANCH ebn-cleanup (branched from master) REVISION DETAIL https://phabricator.kde.org/D25493 To: jhayes, dhaumann Cc: kde-frameworks-devel, LeGast00n, GB_2,

D23457: Port regex search to QRegularExpression

dhaumann added a comment. Any news here? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D23457 To: ahmadsamir, #ktexteditor, dhaumann, cullmann Cc: kde-frameworks-devel, kwrite-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars,

D24982: Small improvements in some XML files

dhaumann added a comment. I think we should decide what to do with this patch, as over time it will get merge conflicts. In general, I hoped we can have some sort of auto detection to disable captures in a smart way. If this is not possible, maybe we should accept this patch?

D25508: Make KF6 cleanup work visible

This revision was automatically updated to reflect the committed changes. Closed by commit R304:c8fa3ab044cb: Make KF6 cleanup work visible (authored by dhaumann). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25508?vs=70264=70266 REVISION DETAIL

D25508: Make KF6 cleanup work visible

dhaumann created this revision. dhaumann added a reviewer: gregormi. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dhaumann requested review of this revision. REVISION SUMMARY This work is simply about removing an unused private member variable that is

D25503: JavaScript: move keywords from TypeScript and other improvements

dhaumann added a comment. I think this is ok. I recognize only now that a much better syntax would have been variable_property but that is off-topic :) If the kateversions are correct, then I'm fine with this. REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D25493: EBN kmoretools cleanup transport protocol

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH ebn-cleanup (branched from master) REVISION DETAIL https://phabricator.kde.org/D25493 To: jhayes, dhaumann Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham,

D17595: Upstream Dolphin's file rename dialog

dhaumann added a comment. In D17595#542142 , @meven wrote: > I received the agreements of all contributors including the original code author, to relicense this code and hence upstream it. Playing the bad guy here: where are these

KF6 Blogs

Hi everyone, could you who currently are at the KF6 sprint blog a bit more? Even if it's just about small details like "app X does not depend on Y anymore and this is how we got rid of the dependency" already is valuable information, since it helps other contributors to not introduce such code

D25485: Remove KIconThemes use

dhaumann added a comment. Makes sense, thanks. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D25485 To: broulik, #frameworks, cullmann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars

D25465: [DropJob] Minor grammar fix in API docs

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Lgtm. REPOSITORY R241 KIO BRANCH l-grammar (branched from master) REVISION DETAIL https://phabricator.kde.org/D25465 To: ahmadsamir, #frameworks, dfaure, apol, dhaumann Cc:

D25276: reStructuredText: Fix inline literals highlighting preceding characters

dhaumann added inline comments. INLINE COMMENTS > rest.xml:14 >--> > mimetype="text/x-rst" version="5" kateversion="2.4" casesensitive="1"> > Please increase the version from 5 to 6. Every time a highlighting file is changed this needs to be done. REPOSITORY R216 Syntax Highlighting

D25328: Always rehiglhight() after definition was changed

dhaumann added a comment. Can't you call `rehighlight()` yourself after calling `setDefinition()`? Btw, tanks for looking into this! REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D25328 To: meven, kossebau, cullmann, vkrause Cc: dhaumann,

D24568: Provide clang-format target with a KDE Frameworks style file

dhaumann added a comment. You can force the current clang format to keep the multi-line if as follows: if (roles.isEmpty() // comment || roles.contains(Notifications::UpdatedRole) || roles.contains(Notifications::ExpiredRole) The comment prevents clang to

D25054: Add string escape characters to PowerShell syntax.

dhaumann added a comment. Looks good to me. Btw, you posted several patches over the last two years. You could also apply for a KDE contributor account and push yourself, if you want. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D25054 To: zrax,

D25054: Add string escape characters to PowerShell syntax.

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Please commit. REPOSITORY R216 Syntax Highlighting BRANCH ps-escape (branched from master) REVISION DETAIL https://phabricator.kde.org/D25054 To: zrax, dhaumann Cc: dhaumann,

D24982: Small improvements in some XML files

dhaumann added a comment. Hm right... too bad. I was hoping to find an automated way to detect this. Since relying on the user to optimize the RegExps will always be suboptimal. @cullmann Do you have any ideas? REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D24983: KateModeMenuList: improve word wrap

dhaumann added a comment. Good, fine with me. Related: Maybe the translation should also be shorter, but the general possible problem persists. Please go ahead. REPOSITORY R39 KTextEditor BRANCH improve-word-wrap REVISION DETAIL https://phabricator.kde.org/D24983 To: nibags,

D24982: Small improvements in some XML files

dhaumann added a comment. > [...] > One option would be to add a **capture** or **dontCapture** attribute to enable or disable captures for RegExpr rules. Also, captures could be enabled or disabled in all RegExpr rules using the group, adding an element for that. But don't we

Re: KSyntaxHighlighting: My changes to data files don’t seem to affect test outputs

What does `ldd build/bin/syntax/testhighlighter_test` tell you? What, if you first source build/prefix.sh? Likely the wrong lib is used. Greetings Dominik Adrian Chaves schrieb am So., 27. Okt. 2019, 17:49: > I emptied data/syntax/rest.xml (made it a valid syntax file that would > mark all

D24983: KateModeMenuList: improve word wrap

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. It's hard to review all the pixel changes with no screenshots. It's even unclear to me what problem exactly is fixed at hand. Given it's all your code, I trust you know what you are

D24982: Small improvements in some XML files

dhaumann added a comment. I wonder if the `?:` optimizations make sense. QRegularExpression has the option `QRegularExpression::DontCaptureOption` to not capture anything. Looking into our code we have: 561 bool RegExpr::doLoad(QXmlStreamReader& reader) 562 { 563

D24981: Modelines: fix end of comment

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

D24939: Meson: more built-in functions and add built-in member functions

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks good to me, thanks! REPOSITORY R216 Syntax Highlighting BRANCH meson (branched from master) REVISION DETAIL https://phabricator.kde.org/D24939 To: jpoelen,

D24620: Windows MSVC compile fix

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Imo we should simply try: We have another two weeks for testing. REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D24620 To: cullmann,

D24568: Provide clang-format target with a common KDE style file

dhaumann added a comment. I'm all for it. This would unify how we can reformat any KDE module, which is very much desirable. Being able to just reformat a patch sounds interesting, too, but can be added later still. Don't let the perfect be the enemy of the good... The example

D24578: Meson: Add a comment section for comment/uncomment with Kate

dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Please increase the version number and commit. REPOSITORY R216 Syntax Highlighting BRANCH meson (branched from master) REVISION DETAIL https://phabricator.kde.org/D24578 To:

D24443: Add a plugin system

dhaumann added a comment. I'd go for a QVector for now. Arguing with Qt6 doesn't sound convincing to me, since Qt6 will take another>=1 year(s). So why try this experiment in public API now? :) REVISION DETAIL https://phabricator.kde.org/D24443 To: nicolasfella, #frameworks, #plasma Cc:

D24415: Add standard icons to support to all entries in QDialogButtonBox

dhaumann added inline comments. INLINE COMMENTS > kstyle.cpp:422 > } > +#if QT_VERSION >= 0x050E00 // Check if Qt version >= 5.14 > +case QStyle::SP_DialogYesToAllButton: Instead of 0x050E00 you could use the macro QT_VERSION_CHECK(5, 14, 0). This would be a bit more self explanatory.

D24403: Small performance improvements suggested by clang-tidy

dhaumann accepted this revision. dhaumann added a comment. I also think it's fine. A next patch could convert the QRegExp to a QRegularExpression. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D24403 To: aacid, cullmann, dhaumann Cc: dhaumann, cullmann,

D24404: Small performance improvements suggested by clang-tidy

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D24404 To: aacid, dhaumann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham,

D24354: Mustache/Handlebars: minor fixes

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

D24399: Pass QDir by const & instead of copy

dhaumann accepted this revision. This revision is now accepted and ready to land. REPOSITORY R244 KCoreAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D24399 To: aacid, dhaumann Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

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

dhaumann added a comment. @graesslin pong? One year passed. REPOSITORY R245 Solid BRANCH gui-instead-of-widgets REVISION DETAIL https://phabricator.kde.org/D13541 To: graesslin, #frameworks, dhaumann, apol, broulik Cc: dhaumann, kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D24378: WordDetect rule: detect delimiters at the inner edge of the string

dhaumann added a comment. I think it's fine as is. The docbook says: Detect an exact string but additionally require word boundaries such as a dot '.' or a whitespace on the beginning and the end of the word. Think of \bstring\b in terms of a regular expression, but it is

D24378: WordDetect rule: detect delimiters at the inner edge of the string

dhaumann added a comment. This looks good to me and as mentioned in D24354 WordDetect is better than RegExpr. +1, but I'd like another review by @cullmann or @vkrause. REPOSITORY R216 Syntax Highlighting REVISION DETAIL

D24326: Add syntax highlighting for RenPy (.rpy)

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

D24354: Mustache/Handlebars: fix delimiters in HTML tags

dhaumann added a comment. Thanks a lot! REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D24354 To: nibags, #framework_syntax_highlighting, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, michaelh, ngraham, bruns,

D24354: Mustache/Handlebars: fix delimiters in HTML tags

dhaumann added a comment. Hm, would it make sense to change this? I.e. if the word itself has a word boundary, also use this? REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D24354 To: nibags, #framework_syntax_highlighting, dhaumann, cullmann Cc:

D24354: Mustache/Handlebars: fix delimiters in HTML tags

dhaumann added a comment. Would it also be an option to fix WordDetect? I always thought WordDetect ignores the string contents, it should only check on the boundaries left and right REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D24354 To: nibags,

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added a comment. In D24262#539317 , @dfaure wrote: > It's actually quite clear in my head, because I imagine the generated class. A captured variable in a lambda becomes a member variable. If it's a capture by value (which is what

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added inline comments. INLINE COMMENTS > kossebau wrote in plugin.cpp:196 > Confirmed by experiments. Still not yet found a document where explicitly it > is mentioned that the copy constructor will be invoked to generate a copy of > the object for any captured variables only being of

D24262: Modernize code: use range-based loops & algorithms in more places

dhaumann added inline comments. INLINE COMMENTS > plugin.cpp:196 > > -QObjectList::ConstIterator it = plugins.begin(); > -for (; it != plugins.end(); ++it) { > -Plugin *plugin = qobject_cast(*it); > -if (plugin && plugin->d->m_library == library) { > -return

  1   2   3   4   5   6   7   8   9   10   >