Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Run][Plugin Manager] Move plugin specific settings under the plugin settings #9655

Closed
mykhailopylyp opened this issue Feb 11, 2021 · 10 comments
Assignees
Labels
Cost-Medium Medium work item - 1-3 Days worth of work. Product-PowerToys Run Improved app launch PT Run (Win+R) Window Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Manager Issues with the PowerToys Run plugin manager

Comments

@mykhailopylyp
Copy link
Contributor

Link to parent issue #5273

It should be possible for plugins to have different settings.
Now we only have one option Disable drive detection warning for the file search plugin
image

@mykhailopylyp mykhailopylyp added the Product-PowerToys Run Improved app launch PT Run (Win+R) Window label Feb 11, 2021
@mykhailopylyp mykhailopylyp self-assigned this Feb 11, 2021
@mykhailopylyp
Copy link
Contributor Author

Here are three examples when we have an indirect reference from PowerLauncher project to plugin projects. We should get rid of them.

// Using OrdinalIgnoreCase since this is internal
var indexer = PluginManager.AllPlugins.Find(p => p.Metadata.Name.Equals("Windows Indexer", StringComparison.OrdinalIgnoreCase));
if (indexer != null)
{
var indexerSettings = indexer.Plugin as ISettingProvider;
indexerSettings.UpdateSettings(overloadSettings);
}

var shell = PluginManager.AllPlugins.Find(pp => pp.Metadata.Name == "Shell");
if (shell != null)
{
var shellSettings = shell.Plugin as ISettingProvider;
shellSettings.UpdateSettings(overloadSettings);
}

if (!plugin.Metadata.Disabled && plugin.Metadata.Name != "Window Walker")
{
_ = PluginManager.QueryForPlugin(plugin, query);
}

@mykhailopylyp
Copy link
Contributor Author

@enricogior
We have two options here:

  • The responsibility of rendering plugin settings is taken by the settings app. That means that we would add information about checkboxes, textboxes, etc. to PowerLauncherPluginSettings. So far we only need to add an array of checkboxes. It is easy to implement but it is not very flexible. It is something between small and medium cost.
  • The responsibility of rendering plugin settings is taken by the plugin project. More precisely settings xaml file is stored in the plugin project and is loaded dynamically in the settings app. It is definitely a big work item.

@enricogior
Copy link
Contributor

@mykhailopylyp
let's go with option 1.

@mykhailopylyp mykhailopylyp added the Cost-Medium Medium work item - 1-3 Days worth of work. label Feb 15, 2021
@htcfreek
Copy link
Collaborator

htcfreek commented Feb 17, 2021

Should we create a spec here how json should be formated and what elements are allowed.

Example (In xml format.):

<settingsGroup name="Result settings">
<checkbox name="test" default="on" settingsJsonString="ShowExampleResult" />
<radioGroup name="Show redults of type">
<radio order=1 name="all" description="test" default="on" settingsJsonString="ShowAllResults" />
<radio order=2 name="games only" description="test" default="on" settingsJsonString="ShowGameResult" />
</radioGroup>
</settingsGroup>

@mykhailopylyp mykhailopylyp added the Status-In progress This issue or work-item is under development label Feb 18, 2021
@mykhailopylyp
Copy link
Contributor Author

Should we create a spec here how json should be formated and what elements are allowed.

@htcfreek
Do you mean spec for the plugin.json file? If yes, it would make a lot of sense when we allow third-party plugins.

@htcfreek
Copy link
Collaborator

htcfreek commented Feb 18, 2021

Should we create a spec here how json should be formated and what elements are allowed.

@htcfreek
Do you mean spec for the plugin.json file?

What I mean is we should strict define what elements are allowed (edit, text, button, ...) and what attributes (name, description, settingValue, ...) can be defined.

I think we should introduce an independent config file for settings and shouldn't integrate them into the general plugin metadata json. (This is more flexible.)

As you see in my example I used a xml for demonstration. Maybe xml is here a better choice.

If yes, it would make a lot of sense when we allow third-party plugins.

Why only for third-party plugins? Why not use it for integrated once too.

@mykhailopylyp
Copy link
Contributor Author

@htcfreek

I think I am not following.

What I mean is we should strict define what elements are allowed (edit, text, button, ...) and what attributes (name, description, settingValue, ...) can be defined.

You mean to define it for additional plugin settings, right? We agreed above that we go with an array of checkboxes for now.

I think we should introduce an independent config file for settings and shouldn't integrate them into the general plugin metadata json. (This is more flexible.)

If you think about it we have not introduced any new settings to plugin.json. Maybe we can create an issue to reduce the number of settings in the metadata.

@htcfreek
Copy link
Collaborator

@htcfreek

I think I am not following.

What I mean is we should strict define what elements are allowed (edit, text, button, ...) and what attributes (name, description, settingValue, ...) can be defined.

You mean to define it for additional plugin settings, right? We agreed above that we go with an array of checkboxes for now.

Yes, thats what I meant.

Okay. This is not much flexible. But let's go with that in the first step. Imo there will come plugins for that a checkbox isn't the best for all settings. (Example: Websearch plugin with selection of search provider.)

I think we should introduce an independent config file for settings and shouldn't integrate them into the general plugin metadata json. (This is more flexible.)

If you think about it we have not introduced any new settings to plugin.json. Maybe we can create an issue to reduce the number of settings in the metadata.

I think we should splitt between plugin metadata definition and complex settings definition. But in the first step we con go with one file.

@htcfreek
Copy link
Collaborator

@enricogior
The new issue for the remaining enhancement idea in this issue is created.

@enricogior enricogior added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Manager Issues with the PowerToys Run plugin manager labels Feb 27, 2021
@crutkas crutkas closed this as completed Mar 4, 2021
@crutkas
Copy link
Member

crutkas commented Mar 4, 2021

Great news! This was resolved in the newly released 0.33 version of PowerToys! Head to https://aka.ms/installpowertoys to grab latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cost-Medium Medium work item - 1-3 Days worth of work. Product-PowerToys Run Improved app launch PT Run (Win+R) Window Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Manager Issues with the PowerToys Run plugin manager
Projects
None yet
Development

No branches or pull requests

4 participants