D9823: Optimize: use QMetaObject::invokeMethod with functor

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY When using this method with a string argument, the method would

D9821: Use QSignalSpy::wait instead of QTest::wait where possible

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This allows the test to finish a bit earlier in the best case.

D9822: Add benchmarks for KDirWatch

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY All benchmarks create a relatively large directory tree and then

D9820: Verify that the file was opened

2018-01-11 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: dfaure. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY The assertion is disabled in release mode, but the verify will

D9812: [Icon Item] Treat sources starting with a slash as local file

2018-01-11 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D9812 To: broulik, #plasma, hein, mwolff Cc: mwolff, plasma-devel, #frameworks, ZrenBot,

D9812: [Icon Item] Treat sources starting with a slash as local file

2018-01-11 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. lgtm in general, but can be cleaned up INLINE COMMENTS > iconitem.cpp:154 > +// If a file:// URL or a absolute path is passed, take the image > pointed by that from disk

D9770: Optimization of byteSize(double size)

2018-01-11 Thread Milian Wolff
mwolff added a comment. 10 calls per second sound fine to me, that shouldn't be a big performance issue at all. Are you measuring performance of a debug build or of a release build? Can you specify the exact commands you are profiling? Is the performance better when you are using KFormat

D9808: fix incorrect emission of signals by kLineEdit

2018-01-11 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > klineedit_unittest.cpp:87 > + > +//if text box is already clear, calling clear() shouldn't emit > +// any more signals typo: if the text box is already clear*ed* > anthonyfieroni wrote in klineedit.cpp:63 > You mean this

D9770: Optimization of byteSize(double size)

2018-01-10 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. Imo this should be using `KFormat::formatByteSize`. https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html#ae7412420b70e2ca935d0ebed6770e313 And if that function

D9766: Use at least the requested width for the argument hint tree

2018-01-09 Thread Milian Wolff
mwolff added reviewers: KDevelop, Kate. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9766 To: mwolff, #kdevelop, #kate Cc: #frameworks, michaelh, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann

D9766: Use at least the requested width for the argument hint tree

2018-01-09 Thread Milian Wolff
mwolff created this revision. Restricted Application added projects: Kate, Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This ensures the code completion tree and the argument hint tree have the same width in the

D9750: ExpandingWidgetModel: find the right-most column based on location

2018-01-09 Thread Milian Wolff
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:7c648c89ed11: ExpandingWidgetModel: find the right-most column based on location (authored by mwolff). REPOSITORY

D9749: Simplify code: return early to reduce indentation depth

2018-01-09 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R39:181c6146d399: Simplify code: return early to reduce indentation depth (authored by mwolff). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9749?vs=24974=24992

D9750: ExpandingWidgetModel: find the right-most column based on location

2018-01-09 Thread Milian Wolff
mwolff added a comment. I'll push it then REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9750 To: mwolff, #kate Cc: cullmann, kfunk, #frameworks, michaelh, kevinapavew, ngraham, demsking, sars, dhaumann

D9750: ExpandingWidgetModel: find the right-most column based on location

2018-01-08 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: Kate. Restricted Application added projects: Kate, Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REVISION SUMMARY This fixes the width calculation for the argument hint

D9749: Simplify code: return early to reduce indentation depth

2018-01-08 Thread Milian Wolff
mwolff created this revision. mwolff added a reviewer: Kate. Restricted Application added projects: Kate, Frameworks. Restricted Application added a subscriber: Frameworks. mwolff requested review of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D9749

D9247: Extend Scripting API to allow executing commands

2018-01-05 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. in general this sounds like a good idea. Also adding getters to the API would be good. there are two typos in your commit message, but stupid phabricator does not let me

Re: Phabricator: How to get the email address of contributors?

2018-01-04 Thread Milian Wolff
ave not necessarily consented to having > their email address made public > > It isn't "bad" software, it just happens to have a workflow weakness > when Arcanist isn't used (which upstream strongly recommend). Yes, one of many workflow weaknesses and I've raised my concerns ab

D9211: Iterate over initializer_lists to avoid mem allocs

2017-12-21 Thread Milian Wolff
mwolff added a comment. much better, but can be somewhat better still. if people don't know the STL, they should learn it. INLINE COMMENTS > katedocument.cpp:109 > +auto it = std::find(list.begin(), list.end(), entry); > +return it == list.end() ? -1 : std::distance(it,

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-21 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R241:a43aea3e: Forward QComboBox signals instead of QComboBox lineedit signals (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7968?vs=24188=24203#toc REPOSITORY R241

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-20 Thread Milian Wolff
mwolff updated this revision to Diff 24188. mwolff added a comment. - expand test to non-editable combo box - fix wrong signal connection (regression introduced when ported to QOverload<>::of) REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D9252: Created 'GroupHiddenRole' for KPlacesModel

2017-12-08 Thread Milian Wolff
mwolff added a comment. thanks lgtm REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9252 To: renatoo, mwolff Cc: #frameworks

D9252: Created 'GroupHiddenRole' for KPlacesModel

2017-12-08 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. you could reduce the code duplication by adding a loop. Note that you can even write something like this: for (auto type : {type1, type2, type3}) { } INLINE COMMENTS

D9241: Emit 'groupHiddenChanged' signal.

2017-12-08 Thread Milian Wolff
mwolff accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D9241 To: renatoo, franckarrecot, mwolff, mlaurent Cc: mwolff, #frameworks

D9241: Emit 'groupHiddenChanged' signal.

2017-12-07 Thread Milian Wolff
mwolff accepted this revision. mwolff added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kfileplacesmodeltest.cpp:1215 > +QList args = groupHiddenSignal.takeFirst(); > +QCOMPARE(args.at(0).toInt(), (int) KFilePlacesModel::SearchForType); > +

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-07 Thread Milian Wolff
mwolff added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7968 To: mwolff, apol, dfaure Cc: #frameworks

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-07 Thread Milian Wolff
mwolff updated this revision to Diff 23604. mwolff added a comment. use QOverload::of as suggested by apol REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7968?vs=23522=23604 BRANCH master REVISION DETAIL https://phabricator.kde.org/D7968 AFFECTED FILES

D8450: User can now hide an entire places group from KFilePlacesView

2017-12-06 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. one minor nit, otherwise lgtm INLINE COMMENTS > kfileplacesmodel.cpp:431 > +QModelIndexList indexes; > +for (int row = 0; row < rowCount(); ++row) { > +const QModelIndex

D8367: Hidding place groups implementation in KFilePlacesModel

2017-12-06 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8367 To: franckarrecot, renatoo, ngraham, ervin, mwolff, mlaurent Cc: mwolff, ngraham, mlaurent, #frameworks

D9189: Do not crash when setting new line edit on an editable combo box

2017-12-06 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R284:fa2c4484d8db: Do not crash when setting new line edit on an editable combo box (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D9189?vs=23508=23549#toc REPOSITORY R284

D9205: QStringList initializer list cleanup

2017-12-05 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. yep this is better than before after all INLINE COMMENTS > kateview.cpp:1385 > > -QStringList l; > - > -l << QStringLiteral("edit_replace") > - <<

D7968: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-05 Thread Milian Wolff
mwolff updated this revision to Diff 23522. mwolff retitled this revision from "WIP: Forward QComboBox signals instead of QComboBox lineedit signals" to "Forward QComboBox signals instead of QComboBox lineedit signals". mwolff edited the summary of this revision. mwolff removed subscribers:

D7968: WIP: Forward QComboBox signals instead of QComboBox lineedit signals

2017-12-05 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > dfaure wrote in kurlrequestertest.cpp:162 > Then use QTest::keyClick to send key events to the widget? I still don't get it to work. This is my currently latest attempt: void KUrlRequesterTest::testComboEditableRequester() {

D9206: Implement a kfile dialog where we can add custom widget

2017-12-05 Thread Milian Wolff
mwolff added a comment. Most of this is just forwarding code from KFileWidget, so we could just use that directly? I mean if our LO integration is going to be the only user of this class, then maybe we should start by adding this code there and only upstream it if we think more people are

D9205: QStringList initializer list cleanup

2017-12-05 Thread Milian Wolff
mwolff added a comment. better than before, but some things could be improved some more INLINE COMMENTS > katedocument.cpp:4518 > +// view variable names > +static const QStringList vvl { > + QStringLiteral("dynamic-word-wrap") even better would be to not name this list

D9189: Do not crash when setting new line edit on an editable combo box

2017-12-05 Thread Milian Wolff
mwolff edited the summary of this revision. REPOSITORY R284 KCompletion REVISION DETAIL https://phabricator.kde.org/D9189 To: mwolff, dfaure Cc: #frameworks

D9189: Do not crash when setting new line edit on an editable combo box

2017-12-05 Thread Milian Wolff
mwolff updated this revision to Diff 23508. mwolff retitled this revision from "Do not crash when completion object gets destroyed underneath us" to "Do not crash when setting new line edit on an editable combo box". mwolff edited the summary of this revision. mwolff removed subscribers:

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-05 Thread Milian Wolff
mwolff updated this revision to Diff 23507. mwolff added a comment. write unit test and document what's going on in more detail REPOSITORY R284 KCompletion CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9189?vs=23473=23507 BRANCH master REVISION DETAIL

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-05 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > mwolff wrote in kcombobox.cpp:317 > can you be more specific, such that I can build a unit test out of this? And > are you saying that `QComboBox::setLineEdit` is deleting the line edit? ah I think I got it now, thanks for the hint! REPOSITORY

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-05 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kcombobox.cpp:317 > KHistoryComboBox is constructed with line edit in, then setLineEdit removes > it with completion object in. can you be more specific, such that I can build a unit test out of this? And are you saying

D9168: Migrate some QRegExps to QRegularExpression

2017-12-05 Thread Milian Wolff
mwolff accepted this revision. mwolff added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > katedocument.cpp:4475 > // found vars, if any > QString s; > future: this could be a ref, quite probably, and capturedRef could be used below >

D9175: Migrate some more QRegExps to QRegularExpression

2017-12-05 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > codecompletionmodelcontrollerinterface.cpp:61 > > -static QRegExp findWordStart(QLatin1String("\\b([_\\w]+)$")); > -static QRegExp findWordEnd(QLatin1String("^([_\\w]*)\\b")); > +static QRegularExpression

D9168: Migrate some QRegExps to QRegularExpression

2017-12-05 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > katewordcompletion.cpp:316 > KTextEditor::Cursor dcCursor; // directional completion search cursor > -QRegExp re; // hrm > +

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-04 Thread Milian Wolff
mwolff added a subscriber: dfaure. mwolff added a comment. This makes KMail's Open With feature work again, but I wonder - is anyone capable of running valgrind? Or figure out where the deletion comes from? @dfaure? REPOSITORY R284 KCompletion REVISION DETAIL

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-04 Thread Milian Wolff
mwolff edited the summary of this revision. REPOSITORY R284 KCompletion REVISION DETAIL https://phabricator.kde.org/D9189 To: mwolff Cc: dfaure, #frameworks

D9189: Do not crash when completion object gets destroyed underneath us

2017-12-04 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY For some reason the 'Open With' dialog opened by KMail triggers the following crash for me: #0 0x68ea4220 in ?? () #1

D7966: KComboBox: Reuse the existing completion object on new line edit

2017-11-30 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D7966#148641, @dfaure wrote: > Just to be sure, your unittest could also test the case of setEditable(false). What should it test then? if it's not editable, there is no line edit, and thus no completion object? REPOSITORY

D7967: KComboBox: Return early when setting editable to previous value

2017-11-30 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R284:11cba47b8718: KComboBox: Return early when setting editable to previous value (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7967?vs=19858=23196#toc REPOSITORY R284

D7966: KComboBox: Reuse the existing completion object on new line edit

2017-11-30 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R284:cf2f5c72d9f3: KComboBox: Reuse the existing completion object on new line edit (authored by mwolff). REPOSITORY R284 KCompletion CHANGES SINCE LAST UPDATE

D7968: WIP: Forward QComboBox signals instead of QComboBox lineedit signals

2017-11-30 Thread Milian Wolff
mwolff added a subscriber: dfaure. mwolff added a comment. right, but how do I unit test this properly? @dfaure? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7968 To: mwolff, apol Cc: dfaure, broulik, apol, #frameworks

Re: Recommended modesetting driver for Intel graphic cards

2017-11-20 Thread Milian Wolff
On Montag, 20. November 2017 23:40:38 CET Milian Wolff wrote: > On Montag, 20. November 2017 17:28:06 CET Martin Flöser wrote: > > Am 2017-11-20 11:59, schrieb Milian Wolff: > > > On Samstag, 18. November 2017 15:34:16 CET Friedrich W. H. Kossebau > > > > >

Re: Recommended modesetting driver for Intel graphic cards

2017-11-20 Thread Milian Wolff
On Montag, 20. November 2017 17:28:06 CET Martin Flöser wrote: > Am 2017-11-20 11:59, schrieb Milian Wolff: > > On Samstag, 18. November 2017 15:34:16 CET Friedrich W. H. Kossebau > > > > wrote: > >> Am Donnerstag, 16. November 2017, 23:24:52 CET schrieb Ingo

D8862: Extend KFilePlacesModel API

2017-11-20 Thread Milian Wolff
mwolff added a comment. the new API and functionality should be covered by tests, it isn't so far as far as I can see. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8862 To: renatoo, dfaure Cc: mwolff, dfaure, ngraham, #frameworks

D8862: Extend KFilePlacesModel API

2017-11-20 Thread Milian Wolff
mwolff requested changes to this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8862 To: renatoo, dfaure, mwolff Cc: mwolff, dfaure, ngraham, #frameworks

D8332: Added baloo urls into places model

2017-11-20 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. lgtm from my side REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8332 To: renatoo, #frameworks, #dolphin, #kde_applications, dvratil, #vdg, ngraham, ervin, mlaurent, dfaure, mwolff Cc: mwolff, dfaure, ervin, usta,

Re: Recommended modesetting driver for Intel graphic cards (was: Re: liquidshell in kdereview)

2017-11-20 Thread Milian Wolff
e xf86ConfigLayout > [12.125] (II) LoadModule: "intel" > [12.127] (WW) Warning, couldn't open module intel > [12.127] (II) UnloadModule: "intel" > [12.127] (II) Unloading intel > [12.127] (EE) Failed to load module "intel" (module does not exist, 0) > [12.127] (II) LoadModule: "modesetting" > [12.127] (II) Loading /usr/lib64/xorg/modules/drivers/modesetting_drv.so I've also recently come across this. According to [1] the performance is supposedly much worse. Is this still true for more recent mesa/kernel versions? [1]: https://www.phoronix.com/scan.php?page=news_item=Intel-DDX-May-Tests Thanks, I'll have to try this out -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.

D8333: fix some indenters from randomly invoking indent

2017-11-19 Thread Milian Wolff
mwolff closed this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D8333 To: brauch, #ktexteditor, cullmann, dhaumann Cc: anthonyfieroni, #frameworks, kevinapavew, ngraham, demsking, cullmann, sars, dhaumann

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2017-11-19 Thread Milian Wolff
mwolff requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D5802 To: intelfx, #kdevelop, #ktexteditor, #kate, mwolff Cc: dhaumann, mwolff, kwrite-devel, #frameworks

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2017-11-19 Thread Milian Wolff
mwolff added a comment. It's not the case, printing and normal are broken now e.g. Couldn't this be fixed by checking what color has the higher contrast - selection or normal? Then pick the one that has the higher contrast. REPOSITORY R39 KTextEditor REVISION DETAIL

D6473: Crash when replacing new lines with spaces

2017-11-19 Thread Milian Wolff
mwolff added a comment. ping? can this be merged? can a unit test be written? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D6473 To: jsalatas, #ktexteditor Cc: mwolff, anthonyfieroni, dhaumann, kfunk, ltoscano, kwrite-devel, #frameworks

D7660: Fix a regression caused by changing backspace key behavior

2017-11-19 Thread Milian Wolff
mwolff resigned from this revision. mwolff added a comment. ping? can this go in? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7660 To: safaalfulaij, #ktexteditor, jgrulich, hein, dhaumann, mwolff Cc: mwolff, ngraham, anthonyfieroni, cullmann, jgrulich,

D7967: KComboBox: Return early when setting editable to previous value

2017-11-19 Thread Milian Wolff
mwolff added a comment. ping? REPOSITORY R284 KCompletion REVISION DETAIL https://phabricator.kde.org/D7967 To: mwolff Cc: #frameworks

D7968: WIP: Forward QComboBox signals instead of QComboBox lineedit signals

2017-11-19 Thread Milian Wolff
mwolff added a comment. ping? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7968 To: mwolff Cc: #frameworks

D8332: Added baloo urls into places model

2017-11-15 Thread Milian Wolff
mwolff added a comment. lgtm, one minor nit, potentially for the future INLINE COMMENTS > kfileplacesmodel.cpp:967 > > +bool KFilePlacesModel::Private::isFileIndexingEnabled() const > +{ this could/should be a free function, not a member, considering its result is cached in a member

D8348: Add a section for removable devices

2017-11-15 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm too REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8348 To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff Cc: mwolff, abetts, mlaurent,

D8450: User can now hide an entire places group from KFilePlacesView

2017-11-15 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. please use arc, or add more context to your patches in the future INLINE COMMENTS > kfileplacesview.cpp:298 > + > +m_disappearingItems.reserve(m_disappearingItems.count() +

D8367: Hidding place groups implementation in KFilePlacesModel

2017-11-15 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesmodel.cpp:70 > +{ > +{ KFilePlacesModel::PlacesType, > QStringLiteral("GroupState-Places-IsHidden") }, > +{

D8348: Add a section for removable devices

2017-11-15 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > mwolff wrote in kfileplacesmodeltest.cpp:181 > the changed list of remote urls that is repeated below could be put into > another helper function done already in https://phabricator.kde.org/D8366 I see, ignore this REPOSITORY R241 KIO

D8348: Add a section for removable devices

2017-11-15 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. the UDI comment needs to be investigated, otherwise lgtm INLINE COMMENTS > kfileplacesmodeltest.cpp:181 > return QStringList() << QDir::homePath() <<

D8348: Add a section for removable devices

2017-11-15 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D8348#163546, @ngraham wrote: > Could you add camera:/ devices to this section too? That way we could also take care of https://bugs.kde.org/show_bug.cgi?id=386060 this isn't done yet, I think REPOSITORY R241 KIO REVISION

D8348: Add a section for removable devices

2017-11-15 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D8348#164718, @ngraham wrote: > In https://phabricator.kde.org/D8348#164713, @abetts wrote: > > > What about something like this as well > > > > F548: Unmount.png > > > > The

D8638: Remove DataModel::roleNameToId

2017-11-03 Thread Milian Wolff
mwolff accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D8638 To: broulik, #plasma, mwolff Cc: plasma-devel, #frameworks, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D8639: Optimize SortFilterModel role names

2017-11-03 Thread Milian Wolff
mwolff accepted this revision. mwolff added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > datamodel.h:141 > protected: > -int roleNameToId(const QString ); //FIXME TODO KF6: This should > have been const. > +int roleNameToId(const QString ) const;

D8434: Created 'remote' section

2017-11-01 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm, but please wait a bit, maybe someone else wants to chime in? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin,

D8434: Created 'remote' section

2017-11-01 Thread Milian Wolff
mwolff added inline comments. INLINE COMMENTS > kfileplacesmodeltest.cpp:784 > +// insert a new network url > +m_places->addPlace(QStringLiteral("My Shared"), QUrl( > QStringLiteral("ftp://192.168.1.1/ftp;)), QString(), QString(), > QModelIndex()); > + please add URLs for the

D8434: Created 'shared' section

2017-11-01 Thread Milian Wolff
mwolff added a comment. But thinking 5s more about it, I personally would say that using "Remote" for the category would be OK for now. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff,

D8434: Created 'shared' section

2017-11-01 Thread Milian Wolff
mwolff added a comment. Well, but if we use `Network` for `remote://` already, then the group should also have this label, no? I don't see an issue with this, really. On the contrary - maybe we could in the future remove the `remote://` link and let the category header react to a click,

D8434: Created 'shared' section

2017-11-01 Thread Milian Wolff
mwolff added a comment. lgtm overall. but I wonder about the naming choice. "Shared" is confusing, to me personally at least. Why not call it "Remote" or "Network"? The reasoning is that "shared" for me is only LAN/WLAN shared stuff, while "remote://" allows arbitrary remote links. I use it

D8043: KDirWatch : make methods virtual

2017-09-28 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. this is not abi compatible, cannot be done -2 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D8043 To: rjvbb, #frameworks, mwolff Cc: mwolff,

Re: Buddies/Tabstops in designer for aggregated KDE widgets?

2017-09-24 Thread Milian Wolff
On Sonntag, 24. September 2017 14:55:37 CEST Milian Wolff wrote: > Hey all, > > I'm trying to get set a sane tabstop order for the record page of hotspot. > To reproduce: > > designer hotspot/src/recordpage.ui > Ctrl + R to preview > -> tab through the form &

Buddies/Tabstops in designer for aggregated KDE widgets?

2017-09-24 Thread Milian Wolff
n't seem to set the tab order (or buddy!) for neither KFilterProxySearchLine nor KMessageWidget. How do I deal with this? Why can one set the buddy + tab order on a KUrlRequester, but not on a KFilterProxySearchLine or KMessageWidget? Thanks -- Milian Wolff m...@milianw.de http://milianw

D7968: WIP: Forward QComboBox signals instead of QComboBox lineedit signals

2017-09-24 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY The QComboBox may not be editable when we connect the signals. When we change the editable flag later on, the signals wouldn't be

D7967: KComboBox: Return early when setting editable to previous value

2017-09-24 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Instead of creating a new line edit, keep the previous one intact. This ensures we don't set a new line edit for code such as the

D7966: KComboBox: Reuse the existing completion object on new line edit

2017-09-24 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY When a KComboBox was created and a completion object configured on it, that setting was lost once the combo box was set to be editable.

D7879: [KConfigGroup] reserve() more and add some C++11

2017-09-21 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. ah ok, and since we don't have a proper byte array view yet, there's nothing you can do (which is sad here!) the other comments from me where suggestions for further improvements, they don't need to hold up this patch REPOSITORY

D7879: [KConfigGroup] reserve() more and add some C++11

2017-09-19 Thread Milian Wolff
mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. it would probably be a good idea to rewrite `KConfigGroupPrivate::serializeList` to not take a `QVariantList`, but rather to use a streaming API. I.e. instead of: void

D7530: KIO: Task should be used in QVector, not QList.

2017-08-27 Thread Milian Wolff
mwolff accepted this revision. This revision is now accepted and ready to land. BRANCH master REVISION DETAIL https://phabricator.kde.org/D7530 To: dfaure, mwolff Cc: #frameworks

D7529: KIO: fix long-standing memory leak on exit.

2017-08-27 Thread Milian Wolff
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm, despite the `delete this`: https://isocpp.org/wiki/faq/freestore-mgmt#delete-this, esp. if you have proper unit tests and ran it through valgrind REPOSITORY R241 KIO BRANCH

D7527: Add mimetype filtering capabilities to KUrlCompletion

2017-08-27 Thread Milian Wolff
mwolff added a comment. In https://phabricator.kde.org/D7527#139670, @dfaure wrote: > Almost perfect ;-) Indeed, should have reviewed it closer myself. Fixed and pushed - thanks for the review David! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7527 To:

D7527: Add mimetype filtering capabilities to KUrlCompletion

2017-08-27 Thread Milian Wolff
This revision was automatically updated to reflect the committed changes. Closed by commit R241:1a92575c73d2: Add mimetype filtering capabilities to KUrlCompletion (authored by mwolff). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7527?vs=18718=18861#toc REPOSITORY R241 KIO CHANGES

D7527: Add mimetype filtering capabilities to KUrlCompletion

2017-08-24 Thread Milian Wolff
mwolff added a comment. Note that the test passes, but I noticed that the KURLCOMPLETION_LOCAL_KIO branch is not covered by unit tests. Setting it even shows that the test is then broken (even without this patch). Also, creating a new thread for every completion request is not a good

D7527: Add mimetype filtering capabilities to KUrlCompletion

2017-08-24 Thread Milian Wolff
mwolff created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This feature already exists in QFileDialog, but is so far missing from KCompletion. This in turn makes KUrlRequester's mimetype filter

ANN: hotspot v1.0.0 released, a GUI for Linux perf (report)

2017-07-06 Thread Milian Wolff
://github.com/KDAB/hotspot/issues Cheers -- Milian Wolff | milian.wo...@kdab.com | Senior Software Engineer KDAB (Deutschland) GmbH KG, a KDAB Group company Tel: +49-30-521325470 KDAB - The Qt Experts smime.p7s Description: S/MIME cryptographic signature

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2017-05-14 Thread Milian Wolff
mwolff added a comment. For solarized you showed the screenshots in your original mail. I'm more concerned about backwards compatibility with other schemes. I.e. yes - we do care about the status quo. Can you give an example for a color scheme where this would break stuff? Then I can also

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2017-05-14 Thread Milian Wolff
mwolff added a comment. Hey there, sorry for the long delay. In general this sounds like a good idea. But what do you mean by "However, just making that change will make Kate/KDevelop incompatible with all existing color schemes"? Can you show some before/after screenshots for an

Re: We need to enable auto close in github pull requests

2017-05-02 Thread Milian Wolff
On Friday, April 28, 2017 3:09:51 PM CEST Albert Astals Cid wrote: > El divendres, 28 d’abril de 2017, a les 11:56:26 CEST, Milian Wolff va > > escriure: > > On Donnerstag, 27. April 2017 17:15:44 CEST Albert Astals Cid wrote: > > > El dilluns, 13 de març de 2017, a

Re: We need to enable auto close in github pull requests

2017-04-28 Thread Milian Wolff
ied here: https://github.com/KDE/heaptrack/pull/9 I want to request that heaptrack is excluded from this bot. I do get mail notifications for heaptrack pull requests and took care of them in the past. I want to keep this contribution channel open. I see no point in not doing so for an extragear

Re: How to install icons for multiple themes

2017-04-20 Thread Milian Wolff
On Donnerstag, 20. April 2017 11:34:39 CEST Matthias Klumpp wrote: > 2017-04-20 11:30 GMT+02:00 Milian Wolff <m...@milianw.de>: > > [...] > > Hey Albert, > > > > sorry for the delay and thanks for the response. The above makes me wonder > > about

Re: How to install icons for multiple themes

2017-04-20 Thread Milian Wolff
On Montag, 17. April 2017 23:36:24 CEST Albert Astals Cid wrote: > El dilluns, 17 d’abril de 2017, a les 22:27:15 CEST, Milian Wolff va escriure: > > Hey all, > > > > how is one supposed to install different versions of icons for different > > icon themes? Say I have

How to install icons for multiple themes

2017-04-17 Thread Milian Wolff
subsystem? Is this in ECM? Am I missing some environment variable? Any help would be appreciated, thanks -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.

<    1   2   3   4   5   6   7   8   9   >