D28544: RecentDocuments Runner: Cleanup and formatting

[Méven Car]

meven added a comment.


  Well @alex https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/31 
was merged, so if you are still up to do it, now is a good time to rebase your 
changes.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Alexander Lohnau]

alex abandoned this revision.
alex added a comment.


  Will continue on gitlab

REPOSITORY
  R120 Plasma Workspace

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

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Méven Car]

meven added a comment.


  To rebase after 
https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/31 lands

REPOSITORY
  R120 Plasma Workspace

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

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Alexander Lohnau]

alex added a comment.


  I didn't knew this patch existed and that it introduces coneptual changes.
  This is just a patch with smaller improvements/cleanup. Because of this I 
suggest waiting for your patch to be merged and than rebasing .

REPOSITORY
  R120 Plasma Workspace

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

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Méven Car]

meven added a comment.


  Regarding the recentdocument krunner, I have a patch to make it rely on 
kactivity data : D26111 . It has been 
waiting for review of a while.
  This patch will introduce a lot of conflict, but I am ok either way, if this 
one get merged first.
  
  Just a small nitpick on may part.

INLINE COMMENTS

> recentdocuments.cpp:107
>  if (folderPath.startsWith(homePath)) {
> -folderPath.replace(0, homePath.length(), 
> QStringLiteral("~"));
>  }

You can replace with `KShell::tildeCollapse` for this if.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Alexander Lohnau]

alex updated this revision to Diff 79244.
alex added a comment.


  Change import

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28544?vs=79243=79244

BRANCH
  recentdocuments_cleanup (branched from master)

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

AFFECTED FILES
  runners/recentdocuments/recentdocuments.cpp
  runners/recentdocuments/recentdocuments.h

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28544: RecentDocuments Runner: Cleanup and formatting

[Alexander Lohnau]

alex created this revision.
alex added reviewers: davidedmundson, ngraham, broulik, meven, apol.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
alex requested review of this revision.

REVISION SUMMARY
  - Instead of early returning in the match method if the list of recent 
documents is empty the matching is suspended.
  - The imports have been cleaned up, redundant Q_UNUSED macros have been 
removed and the formatting has been changed.
  - Instead of getting the action using the id, all the actions are returned 
(in this case just the one that is set in the constructor).
  - The unnecessary comparison of the action has been removed and the replace 
method call uses a more performant overload.

TEST PLAN
  Should compile and show the recent documents.
  To test the suspendMatching: Clear the ~/.local/share/RecentDocuments folder, 
restart krunner.
  There should be no results. Then open a file in dolphin and the file shows up 
as a query result in krunner.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  recentdocuments_cleanup (branched from master)

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

AFFECTED FILES
  runners/recentdocuments/recentdocuments.cpp
  runners/recentdocuments/recentdocuments.h

To: alex, davidedmundson, ngraham, broulik, meven, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart