[KRecentDocument] Consider duplicate entries only based on path, not launched app
AbandonedPublic
Actions

Authored by ngraham on Jun 29 2018, 10:59 PM.

Details

Reviewers

broulik
cfeck

Group Reviewers

Dolphin
Frameworks

Summary

KRecentDocuments currently checks for duplicate entries, but its criteria are very restrictive: not only does the path need to match, but also the app that the path was opened with. As a result, if you open a file with two different apps (say, Kate and KWrite), you get two entries.

From a regular user perspective, these are duplicates--especially since the entries are not visibly badged in any way with what app they will open with, so they look identical. Even if they were, it's not really a useful feature; when you want to access a recently-used document, it's highly likely that you just want to use the default app to open it.

This patch removes the app-matching criteria and considers an entry a duplicate if only its path matches that of an existing entry.

BUG: 379129
FIXED-IN 5.50

Test Plan

Open the same document with both Kate and KWrite. Only one copy of the document shows up in recentdocuments:/ in Dolphin.

Diff Detail

Repository

R241 KIO

Branch

more-liberal-recent-document-duplicate-detection (branched from master)

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 1888
Build 1906: arc lint + arc unit

ngraham created this revision.Jun 29 2018, 10:59 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptJun 29 2018, 10:59 PM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

ngraham requested review of this revision.Jun 29 2018, 10:59 PM

Harbormaster completed remote builds in B477: Diff 36920.Jun 29 2018, 10:59 PM

ngraham edited the summary of this revision. (Show Details)Jun 29 2018, 11:08 PM

ngraham edited the test plan for this revision. (Show Details)

But if I open the file in Kate, it appears in Kate's "Open Recent" menu, even if I previously had opened it in KWrite?

What you say theoretically makes sense, but try it out: the document appears in the Recent Documents menu for both Kate and KWrite, while multi-app aggregator UIs continue to not accumulate duplicates.

cfeck accepted this revision.Jul 18 2018, 9:55 PM

cfeck edited the summary of this revision. (Show Details)

This revision is now accepted and ready to land.Jul 18 2018, 9:56 PM

Thanks! BTW, It's now technically acceptable for the BUG: keyword to go at the beginning. Sysadmins fixed the bug that was preventing this from working before.

@broulik does this patch make sense to you? For me it does not break the case of the same document being opened in multiple programs but it might be nice to get some more testing and a sanity check.

cfeck edited the summary of this revision. (Show Details)Aug 15 2018, 3:25 PM

Still works, and the same document is still available in the Open Recent... menu of multiple apps if you open it in multiple apps. Been using this for a month and a half.

I'm hesitant to land it without more reviews, though.

Rebase on master

Harbormaster completed remote builds in B1888: Diff 39816.Aug 15 2018, 10:52 PM

I cannot reproduce the linked bug. If open the same file in both kate and kwrite I get only one entry in krunner (despite the file having two different X-KDE-LastOpenedWith values in .local/share/RecentDocuments/).

In D13804#310036, @elvisangelaccio wrote:

I cannot reproduce the linked bug. If open the same file in both kate and kwrite I get only one entry in krunner (despite the file having two different X-KDE-LastOpenedWith values in .local/share/RecentDocuments/).

How about in recentdocuments:/ in Dolphin?

Same, there is only one entry there.

Not sure what's going on, maybe somewhere else we are already filtering out duplicates?

Hmm, I can't reproduce it now either. I could have sworn that the issue reproduced for me when I originally tested the bug and submitted the patch, so maybe it was fixed by something else in the meantime. Or maybe I tested it wrong in which case I apologize for wasting your valuable time, Elvis.

No need to apologize, that's what code reviews are for :)

Revision Contents
Changeset List

			Path	Packages
M			src/core/krecentdocument.cpp (5 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	36920	8aa89a7		Jun 29 2018, 10:59 PM	★	★
Diff 2	39816	2a6a174	Rebase on master	Aug 15 2018, 10:51 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
9dff67bd4b62	9b6c396776a8	2a6a174e366c	Nathaniel Graham	[KRecentDocument] Consider duplicate entries only based on path, not launched… (Show More…)	Jun 29 2018, 10:49 PM

Diff 39816

View Options

src/core/krecentdocument.cpp