D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi marked 2 inline comments as done.
tuxxi added a comment.


  Good point about CJK, I had not considered that.

REPOSITORY
  R103 KMenu Editor

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: ognarb, mlaurent, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 52129.
tuxxi added a comment.


  - Use logicalLength for non-latin characters
  - Fix ptr style

REPOSITORY
  R103 KMenu Editor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51565=52129

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ognarb, mlaurent, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi added a comment.


  In D18939#411450 , @ngraham wrote:
  
  > If you use a blank QWidget, you won't have to do that `setContentsMargin()` 
workaround, right?
  
  
  I actually tried this first, I was surprised when the plain `QWidget` still 
had margins; I realized the margins were part of the `QVBoxLayout` rather than 
in the widget containing it. I switched back to a `QFrame` since I've used it 
for this use in the past -- but it really makes no difference in this case, so 
I can switch it to a QWidget if you think that's better style :)

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: mlaurent, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi added a comment.


  Yes, that's exactly what I'm doing. QFrame works fine for doing the same 
thing with the added benefit of easier styling around the edges. But if you 
*really want* I can switch it to a blank QWidget... it makes absolutely no 
difference though.

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51565.
tuxxi added a comment.


  - Fix padding issues

REPOSITORY
  R103 KMenu Editor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51562=51565

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi added inline comments.

INLINE COMMENTS

> ngraham wrote in kmenuedit.cpp:143
> If there's nothing specifically in the style guidelines about it, it's best 
> to follow the existing coding style. Nothing else here uses `auto` with `new` 
> constructors, so we should follow the same style for new code.
> 
> KDE software is multi-generational and it's important that each individual 
> developer not use their own personal preferred style instead of following the 
> existing style because that leads to the whole codebase becoming an 
> inconsistent mess over time. Cleanup can be desirable, but that should happen 
> separately, in its own patch, so it can be discussed on its own merits.

Okay, makes sense. I'll try to be more atomic :)

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51562.
tuxxi marked 8 inline comments as done.
tuxxi added a comment.


  - resolve (hopefully) final nitpicks

REPOSITORY
  R103 KMenu Editor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51561=51562

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi added inline comments.

INLINE COMMENTS

> ngraham wrote in kmenuedit.cpp:30
> Unrelated change.

IMHO it looks cleaner to separate includes into categories, which is what I did 
here.

> ngraham wrote in kmenuedit.cpp:141
> Since the placeholder text says "Search..." we should probably not use the 
> word "Filter" in the tooltip. Also "Type to" isa not really necessary since 
> that's the only thing you can do with a search field. :)
> 
> How about something like "Search through the list of applications below"?

I copied the tooltip text from KOpenWithDialog, but I'll change this one. 
Perhaps we can change it as well

> ngraham wrote in kmenuedit.cpp:143
> No need to use `auto` here

I'm using `auto` to avoid duplicating the type name when using `new` since the 
type is right there; its a habit I got into since clang-tidy recommends it. I 
couldn't find anything in the style guides prohibiting this. If you _really_ 
want, I can stop using `auto`.

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51561.
tuxxi marked 5 inline comments as done.
tuxxi added a comment.


  Fix various nitpicks

REPOSITORY
  R103 KMenu Editor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51545=51561

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51545.
tuxxi marked an inline comment as done.
tuxxi added a comment.


  - search -> search...

REPOSITORY
  R103 KMenu Editor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51540=51545

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51540.
tuxxi added a comment.


  - Removed `.gitignore` changes
  - Switch label to placeholder text

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51468=51540

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18970: update gitignore

[Aidan Sojourner]

tuxxi abandoned this revision.
tuxxi added a comment.


  whoops

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18970

To: tuxxi
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18970: update gitignore

[Aidan Sojourner]

tuxxi created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
tuxxi requested review of this revision.

REVISION SUMMARY
  add basic filtering
  
  added label
  
  fix category expansion when searching
  
  simplify expansion code
  
  use QTreeView methods instead of reinventing the wheel; fix gitignore
  
  change filter threshold to 2 chars, update comments
  
  revert unnesscary changes
  
  update gitignore again since ngraham is a dick
  
  remove label and add placeholder text

REPOSITORY
  R103 KMenu Editor

BRANCH
  search-bar

REVISION DETAIL
  https://phabricator.kde.org/D18970

AFFECTED FILES
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi added a comment.


  Hi ngraham:
  
  OK, can do on the label / placeholder. 
  RE: `.gitignore`, those files are generated by my IDE (CLion). What do you 
suggest I do?

REVISION DETAIL
  https://phabricator.kde.org/D18939

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit: add search/filter bar

[Aidan Sojourner]

tuxxi updated this revision to Diff 51468.
tuxxi added a comment.


  - Switch to using QTreeView's methods for expand/collapse
  - Filter threshold is 2 chars like KOpenWithDialog to avoid cluttering
  - Reverted unnecessary changes (will do a different revision for refactor, 
error fixing, and clang-tidy style)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18939?vs=51457=51468

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  .gitignore
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi, ngraham, #plasma, cfeck
Cc: ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18939: KMenuEdit search/filter

[Aidan Sojourner]

tuxxi created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
tuxxi requested review of this revision.

REVISION SUMMARY
  This diff contains a much-requested feature, adding a search or filter bar to 
KMenuEdit's application tree view.
  
  see: bug 57314 , as well as 
requests on the KDE forums 

TEST PLAN
  KMenuEdit's codebase is a mess; I noticed that there was only one unit test 
run by ctest! I'd love to write more tests in the future, but for this feature 
we don't need much.

REPOSITORY
  R103 KMenu Editor

REVISION DETAIL
  https://phabricator.kde.org/D18939

AFFECTED FILES
  .gitignore
  kmenuedit.cpp
  kmenuedit.h
  treeview.cpp
  treeview.h

To: tuxxi
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart