Differential D9333

Make sure that old URLs do not appear duplicated in places model
ClosedPublic
Actions

Authored by renatoo on Dec 14 2017, 3:44 PM.

Details

Reviewers
mwolff
ngraham
elvisangelaccio
Group Reviewers
Dolphin
Commits
R318:3bf68fd71492: Make sure that old URLs do not appear duplicated in places model
Summary

BUG: 389401

KIO model now provides Baloo URLs; we need to remove the old ones
created by Dolphin places model

BUG: 387888
Depends on D9332

Test Plan

unit test

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.
renatoo created this revision.Dec 14 2017, 3:44 PM
Restricted Application added a subscriber: Dolphin. · View Herald TranscriptDec 14 2017, 3:44 PM
renatoo requested review of this revision.Dec 14 2017, 3:44 PM
renatoo edited the summary of this revision. (Show Details)Dec 14 2017, 3:45 PM
renatoo edited the summary of this revision. (Show Details)
renatoo added a dependency: D9332: Allow filter places based on alternative application name.
renatoo edited the summary of this revision. (Show Details)Dec 14 2017, 3:51 PM
ngraham added inline comments.Dec 14 2017, 4:11 PM
src/panels/places/placesitemmodel.cpp
59

baloonUrls -> balooURLs

621

ballon -> baloo

622

need to remove old ulrs that was visibile only on dolphin to avoid duplication

->

need to remove old URLs that were visible only in Dolphin to avoid duplication

631

baloonUrls -> balooURLs

632

ballon -> baloo

ngraham added inline comments.Dec 14 2017, 5:03 PM
src/panels/places/placesitemmodel.h
171

is -> are

renatoo updated this revision to Diff 23924.Dec 14 2017, 5:05 PM
renatoo marked 5 inline comments as done.

Fixed typo

renatoo updated this revision to Diff 23925.Dec 14 2017, 5:07 PM
renatoo marked an inline comment as done.

Fixed typo

renatoo updated this revision to Diff 23959.Dec 15 2017, 11:30 AM

Updated changes

ervin added a subscriber: ervin.Dec 19 2017, 1:21 PM

Looks fine to me, but I'm not a Dolphin expert so I'll let someone else more knowledgeable accept it.

renatoo updated this revision to Diff 24953.Jan 8 2018, 4:30 PM

Avoid crash while filtering urls

broulik added a subscriber: broulik.Jan 19 2018, 11:48 AM

lgtm

src/panels/places/placesitemmodel.cpp
407

Why that change?

renatoo updated this revision to Diff 25646.Jan 19 2018, 4:39 PM

Solve conflics with mainline

ngraham accepted this revision as: ngraham.Jan 20 2018, 4:17 PM
This revision is now accepted and ready to land.Jan 20 2018, 4:17 PM
ngraham retitled this revision from Make sure that old ulrs does not appear duplicated in places model to Make sure that old URLs do not appear duplicated in places model.Jan 20 2018, 4:18 PM
ngraham edited the summary of this revision. (Show Details)
elvisangelaccio requested changes to this revision.Jan 20 2018, 5:20 PM
elvisangelaccio added a subscriber: elvisangelaccio.
elvisangelaccio added inline comments.
src/panels/places/placesitemmodel.cpp
56

Please fix this comment, it's a leftover of da6f8fe086 but now it doesn't make sense to start the sentence with "Hence".

While at it, let's make AppNamePrefix a QStringLiteral ?

175

please remove the space before the ++

This revision now requires changes to proceed.Jan 20 2018, 5:20 PM
renatoo updated this revision to Diff 25714.Jan 21 2018, 2:06 PM
renatoo marked 2 inline comments as done.

Fixed comments and code style

renatoo marked an inline comment as done.Jan 21 2018, 2:06 PM
elvisangelaccio added inline comments.Jan 21 2018, 2:28 PM
src/panels/places/placesitemmodel.cpp
58–69

hmm why not const QString AppNamePrefix = QStringLiteral("-places-panel"); ?

renatoo updated this revision to Diff 25811.Jan 23 2018, 1:33 PM

Use QString instead of char[]

renatoo marked an inline comment as done.Jan 23 2018, 1:33 PM
elvisangelaccio accepted this revision.Jan 23 2018, 9:53 PM
elvisangelaccio added inline comments.
src/panels/places/placesitemmodel.cpp
58–69

One last nitpick: this is actually a suffix, so while at it I'd rename it AppNameSuffix

This revision is now accepted and ready to land.Jan 23 2018, 9:53 PM
renatoo updated this revision to Diff 25881.Jan 24 2018, 12:47 PM

Renamed variable

renatoo marked an inline comment as done.Jan 24 2018, 12:48 PM
ngraham edited the summary of this revision. (Show Details)Jan 26 2018, 1:16 AM
Closed by commit R318:3bf68fd71492: Make sure that old URLs do not appear duplicated in places model (authored by Renato Araujo Oliveira Filho <renato.araujo@kdab.com>). · Explain WhyJan 26 2018, 2:08 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 25989

View Options

src/panels/places/placesitemmodel.h

Loading...
View Options

src/panels/places/placesitemmodel.cpp

Loading...
View Options

src/tests/placesitemmodeltest.cpp

Loading...