Differential D21600

[DolphinContextMenu] Restore check for whether place already exists
ClosedPublic
Actions

Authored by broulik on Jun 5 2019, 1:22 PM.

Details

Reviewers
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:10d8a5731507: [DolphinContextMenu] Restore check for whether place already exists
Summary

Now that we have a places singleton it adds no real overhead.

Test Plan
  • Added a place to my places panel, opened context menu, didn't get an "Add to places" entry anymore

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Jun 5 2019, 1:22 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptJun 5 2019, 1:22 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
broulik requested review of this revision.Jun 5 2019, 1:22 PM
ngraham added a subscriber: ngraham.Jun 5 2019, 1:23 PM
ngraham added inline comments.
src/dolphincontextmenu.cpp
463–464

Const?

elvisangelaccio added inline comments.Jun 5 2019, 9:35 PM
src/dolphincontextmenu.cpp
466

I know this is what the old code did, but could you try to use

model->match(..., KFilePlacesModel::UrlRole, ...);

like you did in commit 652f57d28152491c4 ?

broulik updated this revision to Diff 59245.Jun 6 2019, 7:00 AM
  • Use QAbstractItemModel::matches
  • Add const

The StripTrailingSlash didn't seem to be neccessary

elvisangelaccio accepted this revision.Jun 6 2019, 9:50 PM
This revision is now accepted and ready to land.Jun 6 2019, 9:50 PM
Closed by commit R318:10d8a5731507: [DolphinContextMenu] Restore check for whether place already exists (authored by broulik). · Explain WhyJun 7 2019, 8:37 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/dolphincontextmenu.cpp (13 lines)

Diff 59316

View Options

src/dolphincontextmenu.cpp

Loading...