D11487: optimization of TextLineData::attribute

jtamate added a comment. In D11487#230895 , @mwolff wrote: > Looking at `TextLineData::addAttribute`, it doesn't seem to sort the data - how can you be sure that the attributes list really is sorted by `offset + length`? I think this should be

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

D11487: optimization of TextLineData::attribute

mwolff added a comment. In D11487#230913 , @jtamate wrote: > In D11487#230895 , @mwolff wrote: > > > Looking at `TextLineData::addAttribute`, it doesn't seem to sort the data - how can you be sure

D11553: Adding Battery Devices Enum

dollinger added a comment. No i don't - my eMail is dollinger.flor...@gmx.de REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11553 To: dollinger, broulik Cc: #frameworks, michaelh, ngraham

D10670: Reduce plasmashell frozen time

hein added a comment. > This revision was not accepted when it landed; it landed in state Needs Review. Weird, David accepted it though? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D10670 To: jtamate, #frameworks, #plasma, mwolff,

D11487: optimization of TextLineData::attribute

jtamate added a comment. I'm sorry, the previous file lines are 10 times longer than expected. This one {F5761669 }can be managed by kate, not only by okteta. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11487 To:

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

nibags added a comment. Then, in such a case, it would be better to make only conservative changes such as: - Change StringDetect to DetectChar or Detect2Chars. - Change AnyChar for DetectChar. - Change RegExpr for StringDetect, DetectChar or Detect2Chars. It would be better to

D11552: [WIP] Handle CJK characters

broulik added a reviewer: hein. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11552 To: michaelh, hein Cc: bruns, lbeltrame, #frameworks, alexeymin, cfeck, ashaposhnikov, michaelh, astippich, spoorun, nicolasfella, ngraham

D11552: [WIP] Handle CJK characters

michaelh added a comment. In D11552#230870 , @alexeymin wrote: > Regarding this - `I don't know if it is really chinese look foreign enough to me anyway.` > Some lines of text in your test script surely look like Japanese Hiragana to me,

D11552: [WIP] Handle CJK characters

michaelh removed reviewers: Baloo, Frameworks, lbeltrame, bruns. michaelh added subscribers: lbeltrame, bruns. Restricted Application added a subscriber: Frameworks. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11552 To: michaelh, #baloo Cc: bruns, lbeltrame,

KDE CI: Frameworks plasma-framework kf5-qt5 SUSEQt5.10 - Build # 108 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20SUSEQt5.10/108/ Project: Frameworks plasma-framework kf5-qt5 SUSEQt5.10 Date of build: Wed, 21 Mar 2018 18:27:07 + Build duration: 15 min and counting JUnit

D11557: remove anchors of item in a layout

davidedmundson updated this revision to Diff 30147. davidedmundson added a comment. Add more cases REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11557?vs=30142=30147 BRANCH origin-master (branched from master) REVISION DETAIL

D11487: optimization of TextLineData::attribute

jtamate added a comment. Uploaded a similar file F5761614: fake.xml.gz It is created using http://interglacial.com/~sburke/pub/rtf2xml.html from a rtf with two images and some text. REPOSITORY R39 KTextEditor REVISION DETAIL

D11491: Don't calculate attribute() twice.

This revision was automatically updated to reflect the committed changes. Closed by commit R39:91999f6e9116: Dont calculate attribute() twice. (authored by jtamate). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11491?vs=29927=30137 REVISION DETAIL

D10670: Reduce plasmashell frozen time

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 R242:33ddaaa23f6b: Reduce plasmashell frozen time (authored by jtamate). REPOSITORY R242 Plasma Framework (Library)

D11557: remove anchors of item in a layout

davidedmundson created this revision. davidedmundson added a reviewer: Plasma. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. davidedmundson requested review of this revision. REVISION SUMMARY This IconItem is in a RowLayout and has

KDE CI: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9 - Build # 83 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20plasma-framework%20kf5-qt5%20FreeBSDQt5.9/83/ Project: Frameworks plasma-framework kf5-qt5 FreeBSDQt5.9 Date of build: Wed, 21 Mar 2018 18:27:07 + Build duration: 4 min 10 sec and counting

D11331: add gaming_input devices and others to Battery

dollinger updated this revision to Diff 30141. dollinger added a comment. Hopefully correct now :D REPOSITORY R245 Solid CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11331?vs=29636=30141 REVISION DETAIL https://phabricator.kde.org/D11331 AFFECTED FILES

D11487: optimization of TextLineData::attribute

mwolff added a subscriber: dhaumann. mwolff added a comment. Looking at `TextLineData::addAttribute`, it doesn't seem to sort the data - how can you be sure that the attributes list really is sorted by `offset + length`? I think this should be done manually there, or at least asserted. The

D10670: Reduce plasmashell frozen time

jtamate added a comment. In D10670#230938 , @hein wrote: > > This revision was not accepted when it landed; it landed in state Needs Review. > > Weird, David accepted it though? Yes. Shouldn't this be reported as a bug to

D10670: Reduce plasmashell frozen time

hein added a comment. I think it's because mwolff did "Requesting changes" and he didn't accept. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D10670 To: jtamate, #frameworks, #plasma, mwolff, davidedmundson Cc: hein, mwolff, plasma-devel,

D10670: Reduce plasmashell frozen time

bcooksley added a comment. Phabricator considers reviews approved only when all individual and group reviewers have approved it. In this instance, nobody approved on behalf of #frameworks and @mwolff has changes requested still. REPOSITORY

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.9 - Build # 158 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.9/158/ Project: Frameworks kio kf5-qt5 FreeBSDQt5.9 Date of build: Wed, 21 Mar 2018 08:50:30 + Build duration: 16 min and counting JUnit Tests Name:

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

nibags edited the summary of this revision. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11543 To: nibags, #framework_syntax_highlighting Cc: #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause, dhaumann

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 179 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/179/ Project: Frameworks kio kf5-qt5 SUSEQt5.10 Date of build: Wed, 21 Mar 2018 08:50:29 + Build duration: 27 min and counting JUnit Tests Name: (root)

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

nibags edited the summary of this revision. nibags added a reviewer: Framework: Syntax Highlighting. nibags added a project: Framework: Syntax Highlighting. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11543 To: nibags, #framework_syntax_highlighting Cc:

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.9 - Build # 29 - Still Unstable!

BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/29/ Project: Frameworks kio kf5-qt5 SUSEQt5.9 Date of build: Wed, 21 Mar 2018 08:50:30 + Build duration: 28 min and counting JUnit Tests Name: (root)

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

nibags created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. nibags requested review of this revision. REVISION SUMMARY Some suggested optimizations of D10621 . **Note:**

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

nibags edited the summary of this revision. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11543 To: nibags, #framework_syntax_highlighting Cc: #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause, dhaumann

D11510: [KPropertiesDialog] Support Checksum tab for any local path

This revision was automatically updated to reflect the committed changes. Closed by commit R241:7d6e4965dfcd: [KPropertiesDialog] Support Checksum tab for any local path (authored by broulik). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11510?vs=2=30100

D11511: [KPropertiesDialog] Support X-KDE-Protocols

broulik added a comment. Doesn't seem to work reliably :/ REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11511 To: broulik, #frameworks, dfaure Cc: michaelh, ngraham

D11487: optimization of TextLineData::attribute

cullmann added a comment. I still am not that convinced that binary search helps for vectors which normally have only << 10-20 elements for normal long text lines. But I would be ok with some range based for variant, as the loop with index increment is even ugly without any speed in mind

D11452: sanitizer: Improve device listing

michaelh added a comment. In D11452#230387 , @smithjd wrote: > Could you please make your tab spacing conform to the style of the > codebase? This should also be fixed in baloodb. baloo follows Kdelibs Coding Style

D11487: optimization of TextLineData::attribute

cullmann added a comment. Ok, that is true, for the "degenerated" very long line case it makes sense to do the binary search. But I don't think you should do that on your own, lower_bound is good enough, we use that for other things like the folding, too, if I am not mistaken. REPOSITORY

D11487: optimization of TextLineData::attribute

jtamate updated this revision to Diff 30117. jtamate edited the summary of this revision. jtamate added a comment. The problem with attribute() is that with a long line and word wrapping enabled, when the long line is shown, it is called as many times as the length of the line, in my

D11487: optimization of TextLineData::attribute

jtamate added a comment. In D11487#230755 , @mwolff wrote: > yes, definitely don't roll your own lower_bound - use the STL provided one. Are you really compiling in release mode while measuring this? Also, I can only repeat myself in saying

D11487: optimization of TextLineData::attribute

mwolff added a comment. yes, definitely don't roll your own lower_bound - use the STL provided one. Are you really compiling in release mode while measuring this? Also, I can only repeat myself in saying that you shouldn't use callgrind for performance measurements anymore, perf/hotspot

D11487: optimization of TextLineData::attribute

mwolff added a comment. In D11487#230791 , @jtamate wrote: > In D11487#230755 , @mwolff wrote: > > > yes, definitely don't roll your own lower_bound - use the STL provided one. Are you really

D8496: Very lightweight cache over device enumeration

davidedmundson abandoned this revision. davidedmundson added a comment. abandoning, going to do something better REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D8496 To: davidedmundson, broulik Cc: #frameworks, michaelh, ngraham

D7563: Add privilegeExecution field to file protocol description

ngraham added a comment. What's the status of this patch? BRANCH master REVISION DETAIL https://phabricator.kde.org/D7563 To: chinmoyr, #frameworks, dfaure Cc: ngraham, elvisangelaccio

D11487: optimization of TextLineData::attribute

mwolff added a comment. When I open your fake file in kwrite, then raise the line length limit and wait for the file to be rendered (which takes quite some time!), perf does not show any performance issues with attribute for me. It only corresponds to ~1.3% of the CPU cycles.

D11487: optimization of TextLineData::attribute

mwolff added a comment. Considering spell checking is involved - can you show a screenshot for how the file looks like for you? There shouldn't be a lot of spell checking going on, or so I hope... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11487 To:

D11569: Improve usability of "Open With" UI

abetts added a comment. Would it look good also that if there were less handlers and they are presented in the same menu that it goes like this: Open With - Gwenview - GIMP I am talking about right justification instead of Open With Gwenview GIMP REPOSITORY R241

D11569: Improve usability of "Open With" UI

anthonyfieroni added inline comments. INLINE COMMENTS > kfileitemactions.cpp:658 > +} else { > +menu ->addSection(i18n("Open With")); > } This introduce one more so menu can grow with 6 items which is much. Why not keep same approach, but we can show

D11487: optimization of TextLineData::attribute

cullmann added a comment. For the sorting/overlapping question: actually the HL code should ensure there are neither overlaps nor unsorted entries. And I think if there are, already now "strange" things happen, as normally the first item would win. REPOSITORY R39 KTextEditor REVISION

D11487: optimization of TextLineData::attribute

anthonyfieroni added inline comments. INLINE COMMENTS > katetextline.cpp:214-216 > +if (first != m_attributesList.cend() && (*first).offset <= pos && pos < > ((*first).offset + (*first).length)) > +{ > +return (*first).attributeValue; Use operator->, it's faster than operator*

D11569: Improve usability of "Open With" UI

ngraham added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kfileitemactions.cpp:658 > This introduce one more so menu can grow with 6 items which is much. Why not > keep same approach, but we can show every time the *preferred* one > > --- > Open with

D11552: [WIP] Handle CJK characters

cfeck added a comment. I have zero knowledge about Baloo, but I can add some comments regarding Unicode. - the four ranges you used are all adjacent, so you could contract to {0x4E00, 0x9FFF} - there are more ranges for CJK characters in the BMP, at least {0x3400, 0x4DBF} would be

D11331: add gaming_input devices and others to Battery

broulik accepted this revision. This revision is now accepted and ready to land. REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11552: [WIP] Handle CJK characters

alexeymin added a comment. Regarding this - `I don't know if it is really chinese look foreign enough to me anyway.` Some lines of text in your test script surely look like Japanese Hiragana to me, especially this one (and tests related to this) echo "otto东到宛平路anna">

D11554: Add warnings to why some actions might be invalid

carewolf added a reviewer: vkrause. REPOSITORY R283 KAuth REVISION DETAIL https://phabricator.kde.org/D11554 To: carewolf, vkrause Cc: #frameworks, michaelh, ngraham

D11331: add gaming_input devices and others to Battery

dollinger added a comment. see D11553 REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11331 To: dollinger, broulik, #plasma Cc: aleksejshilin, #frameworks, michaelh, ngraham

D11553: Adding Battery Devices Enum

broulik accepted this revision. broulik added a comment. This revision is now accepted and ready to land. Thanks. Do you have commit acces? If not I need your email REPOSITORY R245 Solid REVISION DETAIL https://phabricator.kde.org/D11553 To: dollinger, broulik Cc: #frameworks, michaelh,

D11553: Adding Battery Devices Enum

dollinger created this revision. dollinger added a reviewer: broulik. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. dollinger requested review of this revision. REVISION SUMMARY As mentioned in D11331

D11331: add gaming_input devices and others to Battery

broulik added inline comments. INLINE COMMENTS > upowerdevice.cpp:93 > -return (uptype == 2 || uptype == 3 || uptype == 5 || uptype == 6 || > uptype == 7 || uptype == 8); > -default: > -return false; Sorry, just spotted that you removed the `default` case, this will cause

D11554: Add warnings to why some actions might be invalid

carewolf created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. carewolf requested review of this revision. REVISION SUMMARY Otherwise we are only told the action is invalid but not why. REPOSITORY R283 KAuth BRANCH

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

nibags updated this revision to Diff 30172. nibags added a comment. Some changes are undone: - **prolog.xml** : Remove fallthrought, it goes back to RegExpr [1 context]. Use kateversion 3.4. - **rpmspec.xml** : All changes are undone: remove fallthrought [4 contexts]. Use kateversion

D11566: KCapacityBar: set QStyle::State_Horizontal state

cfeck accepted this revision. This revision is now accepted and ready to land. REPOSITORY R236 KWidgetsAddons BRANCH kcapacitybar-state REVISION DETAIL https://phabricator.kde.org/D11566 To: zzag, cfeck, elvisangelaccio Cc: #frameworks, michaelh, ngraham

D11529: Balooctl: Deindex unfound files with check command.

smithjd updated this revision to Diff 30173. smithjd added a comment. Fix and optimize. REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11529?vs=30048=30173 BRANCH master-filedeindexer REVISION DETAIL https://phabricator.kde.org/D11529 AFFECTED FILES

D11568: cleanup test files

davidedmundson created this revision. davidedmundson added a reviewer: Plasma. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. davidedmundson requested review of this revision. REVISION SUMMARY Otherwise it can cause an issue when the

D11569: Improve usability of "Open With" UI

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11569 To: ngraham, rkflx, #vdg, #frameworks, #dolphin Cc: michaelh, ngraham

D11566: KCapacityBar: set QStyle::State_Horizontal state

zzag created this revision. zzag added reviewers: cfeck, elvisangelaccio. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. zzag requested review of this revision. REVISION SUMMARY QStyleOptionProgressBarV2 has been deprecated in favor of

D11566: KCapacityBar: set QStyle::State_Horizontal state

zzag added a comment. otherwise, StatusBarSpaceInfo(in Dolphin) would render as following F5761872: foo.png REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D11566 To: zzag, cfeck, elvisangelaccio Cc: #frameworks,

D11569: Improve usability of "Open With" UI

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D11569 To: ngraham, rkflx, #vdg, #frameworks, #dolphin Cc: michaelh, ngraham

D11569: Improve usability of "Open With" UI

ngraham created this revision. ngraham added reviewers: rkflx, VDG, Frameworks, Dolphin. Restricted Application added a project: Frameworks. ngraham requested review of this revision. REVISION SUMMARY This patch adjusts the Open With UI to improve the strings and display apps inline (without a

D11569: Improve usability of "Open With" UI

ngraham added a comment. Possible change: In the inline case where there are fewer than 5 apps, we could use a header entitled "Open With" and then remove that text from all the menu items, to prevent it from being duplicated up to 5 times. REPOSITORY R241 KIO REVISION DETAIL

D11569: Improve usability of "Open With" UI

ngraham updated this revision to Diff 30182. ngraham added a comment. Use a header for 1-4 apps; it looks better and there's no redundant text REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11569?vs=30181=30182 BRANCH open-with-usability (branched from

D11552: [WIP] Handle CJK characters

michaelh created this revision. michaelh added reviewers: Baloo, Frameworks, lbeltrame, bruns. Restricted Application added projects: Frameworks, Baloo. michaelh requested review of this revision. REVISION SUMMARY This patch is a request for comments. 1. Is this approach viable? 2. Who

D10251: [RFC] Use mipmap filtering in window thumbnails

davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D10251 To: fredrik, #plasma, #vdg, davidedmundson Cc: progwolff, broulik, ngraham, hein, plasma-devel, #frameworks,

Re: Experimental class in baloo

On 21.03.2018 01:16, David Edmundson wrote: On Tue, Mar 20, 2018 at 9:43 AM, Michael Heidelbach > wrote: Hi! I've recently introduced a new class for baloo

D11487: optimization of TextLineData::attribute

jtamate updated this revision to Diff 30134. jtamate added a comment. > Huh, just specify -DCMAKE_INSTALL_PREFIX to the same path where you install the KF5 devel packages to, as a simple fix. Thanks, that worked. > Are you really compiling in release mode while measuring this?