D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

This revision was automatically updated to reflect the committed changes. Closed by commit R120:e02f3fcb1347: [Bookmarks Runner] Remove duplicate results for bookmarks (authored by bruns). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15357?vs=42615&

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

broulik accepted this revision. broulik added a comment. This revision is now accepted and ready to land. Somehow the usage of `equal_range` and `keyValueBegin` looks quite convoluted but if it works... ship it REPOSITORY R120 Plasma Workspace BRANCH T9626 REVISION DETAIL https://phab

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. I would really appreciate if we could get this into the imminent Plasma release. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: davidedmundson, zzag, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesli

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. qdbus-qt5 org.kde.krunner /App org.kde.krunner.App.querySingleRunner bookmarks kde Current state: F6298174: Bookmarks_Current.png Deduplicated: F6298173: Bookmarks_Deduplicated.png

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns updated this revision to Diff 42615. bruns added a comment. add inline comments to code REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15357?vs=41227&id=42615 BRANCH T9626 REVISION DETAIL https://phabricator.kde.org/D15357 AFFECTED FIL

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. Is there anything missing? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: davidedmundson, zzag, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. Ping! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: davidedmundson, zzag, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. Ping! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: davidedmundson, zzag, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. @davidedmundson - good to go? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: davidedmundson, zzag, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, a

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added a comment. In D15357#322812 , @davidedmundson wrote: > RunnerContext::addMatches has its own duplicate check based on match.id > > We're not explicitly setting that currently. > I think we can just do match.setId(url+title); a

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

davidedmundson added a comment. RunnerContext::addMatches has it's own duplicate check based on match.id We're not explicitly setting that currently. I think we can just do match.setId("bookmarks"+url+title); and have everything handled auto-magically. If that does work it will be

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added inline comments. INLINE COMMENTS > zzag wrote in firefox.cpp:136 > Please use prefix increment. If there are more required changes, yes, else - no. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: zzag, ngraham, plasma-

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

zzag added inline comments. INLINE COMMENTS > bruns wrote in firefox.cpp:136 > It neither guarantees prefix is more efficient than postfix. Please use prefix increment. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: zzag, ngraham

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added inline comments. INLINE COMMENTS > zzag wrote in firefox.cpp:136 > Does the C++ standard guarantee that? It neither guarantees prefix is more efficient than postfix. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: zzag

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

zzag added inline comments. INLINE COMMENTS > bruns wrote in firefox.cpp:136 > Because modern compilers are clever enough do avoid any temporaries Does the C++ standard guarantee that? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma C

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns added inline comments. INLINE COMMENTS > zzag wrote in firefox.cpp:136 > Why postfix increment? Because modern compilers are clever enough do avoid any temporaries REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D15357 To: bruns, #plasma Cc: zzag, ngraha

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

zzag added inline comments. INLINE COMMENTS > firefox.cpp:136 > + > +for (auto result = uniqueResults.constKeyValueBegin(); result != > uniqueResults.constKeyValueEnd(); result++) { > +BookmarkMatch bookmarkMatch(m_favicon, term, (*result).second, > (*result).first); Why postfix in

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns updated this revision to Diff 41227. bruns added a comment. rebase REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15357?vs=41218&id=41227 BRANCH T9626 REVISION DETAIL https://phabricator.kde.org/D15357 AFFECTED FILES runners/bookmark

D15357: [Bookmarks Runner] Remove duplicate results for bookmarks

bruns created this revision. bruns added a reviewer: Plasma. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. bruns requested review of this revision. REVISION SUMMARY An entry from the moz_places db may have several referring entries in the moz_bookmarks db, i.e. where