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
Lint Skipped
Unit
Unit Tests Skipped
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
464 ↗(On Diff #59194)

Const?

elvisangelaccio added inline comments.Jun 5 2019, 9:35 PM
src/dolphincontextmenu.cpp
467 ↗(On Diff #59194)

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 59245

View Options

src/dolphincontextmenu.cpp

Loading...