Add a new platform constraint for filtering plugins
AbandonedPublic

Authored by ervin on Oct 12 2017, 11:37 AM.

Details

Reviewers
aacid
apol
Summary

Some plugins don't necessarily make sense in all types of systems (e.g.
"saveasplugin" looks foreign on desktop applications which have that in
their file menu already). That's why this commit introduce a new type of
constraint named "platform" which can be either "mobile" or "desktop".

For a given AlternativesModel a platform can be forced via setInputData
like for other constraints, but if none is forced the library applies a
default picked at build time (typically: Android, iOS or Windows Phone
get the "mobile" platform, everything else is "desktop").

This commit also restricts the "saveasplugin" to be displayed only on
mobile platforms.

Diff Detail

Repository
R495 Purpose Library
Lint
Lint Skipped
Unit
Unit Tests Skipped
ervin created this revision.Oct 12 2017, 11:37 AM
apol added a comment.Oct 12 2017, 2:26 PM

What makes you think it's something for mobile?

I in fact added this in the first place in KDevelop (where we use it to save patches locally instead of phabricator/reviewboard), I've also used it to save pastes as local files.

I have the impression that what makes saveas special is that the applications that want to filter it out it is because they (possibly correctly) have a specific save-as dialog (use-case both for Okular and Spectacle, at least). That's why I suggested the explicit blacklisting API.

Can you think of other platform-specific plugins we might have? I would expect that if a plugin doesn't apply to some specific platform it would never end up installed in the first place (unless it doesn't fit because convergence, depends on the definition of platform).

ervin added a comment.Oct 13 2017, 8:08 AM

Perhaps I should have named that form-factor. I personally think it makes sense to go about it this way, especially in the case of convergence where the form factor might change and thus the save as action appear in different places (although granted the current implementation doesn't go that far). Beside on desktop a "Save As" action in a share menu is totally foreign while on mobile it's the other way around.

The problem I see with explicit blacklisting of plugins by id is that it means that for most (of course not all) desktop applications people would have to blacklist as soon as they use Purpose::Menu because they have their own "Save As" action. So it feels wrong on desktop in 80% of the cases.

Or an alternative would be to go with the blacklist, but have it contain save as by default on desktop... which will create its own set of problems: as in remembering to remove it from the blacklist when you're in the KDevelop case (although I suspect it's much less common than the other way around).

apol added a comment.Oct 13 2017, 10:58 AM

I find it a bit convoluted. I would prefer to add the blacklisting while being less automatic it gives the developer the power to actually say what he wants.

To make sure this API doesn't break some use-cases in kdevelop and the quick share plasmoid we would have to Purpose::AlternativesModel::setDefaultPlatform(QStringLiteral("mobile")); while not being mobile, which feels like a workaround as well.

Maybe if we see the use-cases of the blacklisting extending everywhere we'll be able to find what's the semantics that we are looking for.

So what about the blacklist API with a default of {"saveasplugin"} in the case of a desktop build? It already gets de facto a default with the changes from Dan.

Just thinking about "should work in most cases out of the box". So yes indeed would need to remove saveasplugin from that blacklist in some of the desktop applications, but it's likely not the majority of them.

apol added a comment.Oct 16 2017, 6:07 PM
In D8262#154907, @ervin wrote:

So what about the blacklist API with a default of {"saveasplugin"} in the case of a desktop build? It already gets de facto a default with the changes from Dan.

Just thinking about "should work in most cases out of the box". So yes indeed would need to remove saveasplugin from that blacklist in some of the desktop applications, but it's likely not the majority of them.

Makes sense, or maybe an additional input. I wouldn't include whatever the sysadmin decided, though. If the admin decides that "nextcloud" isn't acceptable, we shouldn't let the app developer to add it back.

ervin abandoned this revision.Oct 17 2017, 8:52 AM

OK, looks like we got an agreement so abandoning that change, a new one will be uploaded.