Add Tags browser places item to Dolphin and file pickers by default if Baloo is enabled
AbandonedPublic

Authored by ngraham on Sep 3 2017, 12:04 AM.

Details

Summary

BUG: 182367

Add a Tags places item (tags:/) to Dolphin and file pickers by default if Baloo is enabled

Test Plan

Tested this diff in up-to-date KDE Neon. If Baloo is enabled, by default a tags:/ places item appears in the Places view in both Dolphin and all file pickers:

Diff Detail

Repository
R241 KIO
Lint
Lint Skipped
Unit
Unit Tests Skipped
ngraham created this revision.Sep 3 2017, 12:04 AM

This is similar to the change in https://phabricator.kde.org/D7446. These hidden features are so powerful; we need to reveal them!

The one issue that I foresee with adding this is if a novice user clicks on it before any tags have been created, they might be slightly confused to see a blank view (since there are no tags). If this is approved, I'll work on adding some explanatory text to display instead of a blank white background. Something like "Tags are a way to categorize your files and folders for quick access. Any tags you create will appear here."

cfeck added a subscriber: cfeck.Sep 3 2017, 12:59 AM

I tried entering "tags:/" in Dolphin, but got "Invalid protocol", most likely because I have Baloo disabled. Either the error message needs to be more clear, or the item should be hidden for disabled Baloo.

Oh good point, I forgot that this won't work if Baloo is disabled. I'll guard it behind an #ifdef, which appears to be the standard way to do this, looking through Dolphin's code.

Since I'm proposing to change a default, the #ifdef will only avoid creating the bookmark on first launch for systems where Baloo is disabled at the distro-level. For users who opt to turn Baloo off themselves, I think it's safe to assume they're technically competent enough to understand the ramifications of that decision. But yes, it would be nice if there was a better error message. With Baloo disabled, I assume your Information panel also doesn't show Tags, right?

ngraham updated this revision to Diff 19112.Sep 3 2017, 1:08 AM
ngraham retitled this revision from Add Tags browser places item to Dolphin and file pickers by default to Add Tags browser places item to Dolphin and file pickers by default if Baloo is enabled.
ngraham edited the summary of this revision. (Show Details)
elvisangelaccio requested changes to this revision.Sep 3 2017, 8:19 AM
elvisangelaccio added a subscriber: elvisangelaccio.

Note that this only works if there are no custom places yet (probably not a big deal).

src/filewidgets/kfileplacesmodel.cpp
110

Doesn't seem to be defined in KIO.

You can use KProtocolInfo::isKnownProtocol(QStringLiteral("tags"))

This revision now requires changes to proceed.Sep 3 2017, 8:19 AM

Note that this only works if there are no custom places yet (probably not a big deal).

Yes, the idea here is to change the default setting for new installs, and this code is only run the first time, before the user has added any custom places.

Thanks for pointing out the #ifdef HAVE_BALOO issue. I'll admit wasn't actually able to test this due to the Frameworks 5.38.0 bump yesterday which caused a few build issues, and I couldn't get all the dependencies to build. I'll make the requested change, and test when I'm able to sort out the build issues, or when Neon issues package updates for version 5.38.0.

ngraham updated this revision to Diff 19135.Sep 3 2017, 1:49 PM
ngraham edited edge metadata.

Used a different mechanism for gating this code on Baloo being active.

ltoscano edited edge metadata.Sep 3 2017, 1:52 PM

Oh good point, I forgot that this won't work if Baloo is disabled. I'll guard it behind an #ifdef, which appears to be the standard way to do this, looking through Dolphin's code.

Since I'm proposing to change a default, the #ifdef will only avoid creating the bookmark on first launch for systems where Baloo is disabled at the distro-level. For users who opt to turn Baloo off themselves, I think it's safe to assume they're technically competent enough to understand the ramifications of that decision. But yes, it would be nice if there was a better error message. With Baloo disabled, I assume your Information panel also doesn't show Tags, right?

I wouldn't assume this: what about the situation when Baloo is compiled in, but disabled by default? The point is that it's not always:

  • Baloo compiled out -> no nned for the bookmarks
  • Baloo compiled in -> enabled by default, the user may have turned it off

because (for example) the distribution may have compiled it but disabled by default.

ngraham marked an inline comment as done.Sep 3 2017, 3:33 PM

The latest change should take care of that concern; I changed the #ifdefto if (KProtocolInfo::isKnownProtocol(QStringLiteral("tags"))), which should cover the case where Baloo was disabled after compilation but before Dolphin was first opened.

ngraham edited the test plan for this revision. (Show Details)Sep 4 2017, 6:28 PM
ngraham added a reviewer: aacid.

Any remaining objections?

Maybe D7700 is better? We should choose one here, I guess

https://phabricator.kde.org/D7700 is definitely better. My patch here was just a quick-and-dirty way to get it quickly. We can close mine if that gets committed.

ngraham abandoned this revision.Mar 19 2019, 12:29 AM
ngraham added a subscriber: nicolasfella.

Doesn't make sense; @nicolasfella's new Places Panel section makes more sense to put in once it's ready.

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald TranscriptMar 19 2019, 12:29 AM