[DSC] Implement Microsoft.PowerToys.Configure DSCResource & winget support #30918

yuyoyuppe · 2024-01-12T20:10:14Z

Summary of the Pull Request

Allows to configure most of PowerToys settings using winget configure like this:

# yaml-language-server: $schema=https://aka.ms/configuration-dsc-schema/0.2
properties:
  resources:
    - resource: PowerToysConfigure
      directives:
        description: Configure PowerToys
      settings:
        ShortcutGuide:
          Enabled: false
        FancyZones:
          Enabled: true
          FancyzonesEditorHotkey: "Shift+Ctrl+Alt+F"
  configurationVersion: 0.2.0

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

htcfreek · 2024-02-07T17:28:46Z

I don't know how this will work inside PowerToys code. But we have to make sure that the Group Policies always in any case have the highest priority!

yuyoyuppe · 2024-02-07T17:44:40Z

@htcfreek good point, thanks. In general, we must ensure that when a setting value in .json is edited - it's correctly applied and respected by both Settings and a module.

I'm afraid we'll encounter a number of bugs and we'll have to solve it in a setting-by-setting manner.

…e support

yuyoyuppe · 2024-03-20T01:06:52Z

Implemented the remaining TODO items and merged the latest main.

jaimecbernardo

Looks like lots of work went into refactoring some weird behaviors we had. Great work! I've done a first review and have some nits / questions.

.gitignore

jaimecbernardo · 2024-03-21T10:21:56Z

src/dsc/Microsoft.PowerToys.Configure/Microsoft.PowerToys.Configure.psd1

+Copyright = '(c) Microsoft Corporation. All rights reserved.'
+
+# Description of the functionality provided by this module
+# Description = ''


Do we need to fill out any of these to comply? Can you please check for some advisory on this?

sent an email with the question regarding this

jaimecbernardo · 2024-03-21T10:23:27Z

src/dsc/Microsoft.PowerToys.Configure/examples/disableAllModules.dsc.yaml

+      directives:
+        description: Configure PowerToys
+      settings:
+        Debug: true


Is this one needed? Why is Debug on this sample? It seems to be on every sample. I guess the samples don't need debug, since that's only for debugging.

Debug is used so it's possible to test DSC without building an installer and performing an installation each time you change something. Changed to false.

@yuyoyuppe , why change it to false? It can be deleted from the file too, right?

It can be deleted, yes. It's convenient to have it here though.

If these are the samples we are providing for people to look, please remove the Delete.

jaimecbernardo · 2024-03-21T10:28:25Z

src/dsc/Microsoft.PowerToys.Configure/examples/configureLauncherPlugins.dsc.yaml

+        Debug: true
+        PowerLauncher:
+          Enabled: true
+          AdditionalProperties:


Why are plugins inside Additional Properties? That looks a bit weird.

Plugins have multiple things that do not comply with the rest of the settings:

They're stored separately from Properties field unlike all other settings, see PowerLauncherSettings

They are stored in a IEnumerable which makes identifying them difficult, thus the inferring of "key" property (Name only at the moment) in SetAdditionalSettingsCommandLineCommand.Execute. Also, it's not clear whether we should append/combine/overwrite the elements in IEnumerable.

So yeah, currently they exist as a kind of special case due to their structure and extensibility.

@yuyoyuppe
For policies we use the plugin id as identification because this is the only thing that is 100% unique. From what I have in mind PowerLauncher deduplicates based on the id and newest version/date.

Why is it named "AdditionalProperties", though? Is that the name in the json file?

That's just the name I've picked. We could change it to something else.

So ti means we can just have "Plugins" instead of "Additional Properties>Plugins"? That would be preferred.

The way it works is more general than that though 🤔. Currently, it's a bit limited in terms of the supported types, but the logic could be extended to support all the properties, i.e. not only Plugins for PowerRun, but for the rest of the properties as well.

doc/devdocs/settingsv2/dsc-configure.md

jaimecbernardo · 2024-03-21T10:46:18Z

src/settings-ui/Settings.UI.Library/ImageResizerProperties.cs

@@ -69,6 +70,7 @@ public ImageResizerProperties(Func<string, string> resourceLoader)
        public StringProperty ImageresizerFileName { get; set; }

        [JsonPropertyName("imageresizer_sizes")]
+        [CmdConfigureIgnoreAttribute]


So right now we're unable to do the ImageResizer sizes we want from DSC. Can we please document in the .md the current settings we're unable to configure at this moment? This looks like one that would be useful. Not saying we do it for V1, but we should keep track of what we're missing.

jaimecbernardo · 2024-03-21T10:47:42Z

src/settings-ui/Settings.UI.Library/KeyboardManagerProperties.cs

        public GenericProperty<string> ActiveConfiguration { get; set; }

        // List of all Keyboard Configurations.
        [JsonPropertyName("keyboardConfigurations")]
+        [CmdConfigureIgnoreAttribute]
        public GenericProperty<List<string>> KeyboardConfigurations { get; set; }


Are we able to do KBM mappings through the DSC currently?

Yes, doing multilevel generics needs additional handling.

Can we please have a sample for this added, then? ;) Thank you!

Eh, meant to say we can't do it yet. 😐

Please add a list of the Settings we don't currently support setting to the documentation.

src/settings-ui/Settings.UI.Library/ShortcutGuideProperties.cs

src/dsc/Microsoft.PowerToys.Configure/examples/enableAllModules.dsc.yaml

jaimecbernardo · 2024-03-21T11:00:36Z

src/dsc/Microsoft.PowerToys.Configure/examples/enableAllModules.dsc.yaml

+          Enabled: true
+        VideoConference:
+          Enabled: true
+        MeasureTool:


On Settings we're showing different names for these, so we might need to add comments to the samples so people can understand what we're talking about for the ones that don't look like the names in the product. Thinking Measure Tool is actually Screen Ruler and Power Ocr is actually Text Extractor, for example. Or can we have aliases for these?

My idea was to add the documentation for all the settings. Having an alias is a good idea as well, I'll take a look

htcfreek · 2024-03-21T13:02:48Z

src/dsc/Microsoft.PowerToys.Configure/examples/configureLauncherPlugins.dsc.yaml

+            Plugins:
+              - Name: "Calculator"
+                Disabled: true


Do we only support disabled state?

we support both states, that's just naming of the plugin properties here.

Unclear asked. Is this the only setting you can set?

I think we should support other options as well. Not the nested objects though. I'll compose a list of settable options for all modules and create a PR in https://github.com/MicrosoftDocs/windows-dev-docs/blob/docs/hub/powertoys/index.md

# Conflicts: # PowerToys.sln

htcfreek · 2024-04-01T10:31:03Z

src/dsc/Microsoft.PowerToys.Configure/examples/allSettings.dsc.yaml

+          IgnoreHotkeysInFullscreen: false
+          ClearInputOnLaunch: false
+          TabSelectsContextButtons: false
+          Theme: HighContrastBlack


Wonder about this value. Is it matching to a selectable value in the settings ui?

htcfreek · 2024-04-01T10:31:33Z

src/dsc/Microsoft.PowerToys.Configure/examples/allSettings.dsc.yaml

+          SortByUsageFrequency: false
+          StartSelectionFromTheLeft: false
+
+        PowerLauncher:


Missing the plugin sample for PT Run?

Looking at this the all Setting sample contains mostly noise. I think it's better to remove it at this point 🤷

jaimecbernardo

There's still some changes that are required, I think. Or oversights in the changes.

installer/PowerToysSetup/Core.wxs

jaimecbernardo · 2024-03-25T11:11:18Z

installer/PowerToysSetup/Core.wxs

+        <Directory Id="PowerShellModulesFolder" Name="Modules">
+          <Directory Id="PowerToysDscFolder" Name="Microsoft.PowerToys.Configure">
+            <Directory Id="PowerToysDscVerFolder" Name="$(var.Version)">
+              <Component Id="PowerToysDSC" Win64="yes" Guid="4A033E3B-6590-43FD-8FBD-27F9DF557F7F">


Can we make the component not fail the installation if the component is not able to be installed?

src/dsc/Microsoft.PowerToys.Configure/examples/allSettings.dsc.yaml

jaimecbernardo · 2024-04-01T10:16:26Z

src/dsc/Microsoft.PowerToys.Configure/examples/configureLauncherPlugins.dsc.yaml

+          Enabled: true
+          Plugins:
+            - Name: "Calculator"
+              Disabled: true


Are other fields configurable here too for the plugins?

src/settings-ui/Settings.UI.Library/HostsProperties.cs

src/settings-ui/Settings.UI.Library/MouseHighlighterProperties.cs

src/settings-ui/Settings.UI.Library/PowerLauncherProperties.cs

jaimecbernardo · 2024-04-01T10:29:44Z

src/settings-ui/Settings.UI.Library/PowerLauncherProperties.cs

@@ -49,6 +58,7 @@ public class PowerLauncherProperties
        public Theme Theme { get; set; }

        [JsonPropertyName("show_plugins_overview")]
+        [CmdConfigureIgnore]
        public int ShowPluginsOverview { get; set; }


Why is this one ignored too?

It's there on Settings, but it's an int instead of an Enum. We need to look into how to support it later on.

This reverts commit 6da3d6c.

jaimecbernardo

LGTM with the latest changes. Good enough for a V1, at least. Thank you. Now waiting for CI to run before merging.

htcfreek · 2024-04-02T06:14:23Z

@yuyoyuppe , @jaimecbernardo
Ist the following run plugin setting configurable using dsc? If yes, please block this as it is an internal state configuration field.

https://github.com/microsoft/PowerToys/blob/main/src%2Fsettings-ui%2FSettings.UI.Library%2FPowerLauncherPluginSettings.cs#L23

yuyoyuppe requested review from jaimecbernardo and donlaci January 12, 2024 20:10

yuyoyuppe assigned yuyoyuppe and donlaci Jan 12, 2024

This comment has been minimized.

Sign in to view

yuyoyuppe force-pushed the dsc_configure_module branch from 3c69103 to c99dd8e Compare March 20, 2024 00:59

This comment has been minimized.

Sign in to view

[DSC] Microsoft.PowerToys.Configure module + winget configuration fil…

Loading
Loading status checks…

d7663d6

…e support

yuyoyuppe force-pushed the dsc_configure_module branch from c99dd8e to d7663d6 Compare March 20, 2024 01:02

yuyoyuppe marked this pull request as ready for review March 20, 2024 01:06

f: fix for an incorrect directory id reference

Loading
Loading status checks…

f8e4737

yuyoyuppe changed the title ~~[WIP][DSC] Implement Microsoft.PowerToys.Configure DSCResource & winget support~~ [DSC] Implement Microsoft.PowerToys.Configure DSCResource & winget support Mar 20, 2024

f: update comment

Loading
Loading status checks…

cd69d09

jaimecbernardo suggested changes Mar 21, 2024

View reviewed changes

f: address review comments

Loading
Loading status checks…

92d373f

htcfreek reviewed Mar 21, 2024

View reviewed changes

yuyoyuppe and others added 7 commits March 21, 2024 16:09

f: file locksmith bug fix

Loading
Loading status checks…

251e967

f: add explorer preview switches in samples

Loading
Loading status checks…

1275da5

f: remove debug

Loading
Loading status checks…

f3fce0e

Sign DSC files

Loading
Loading status checks…

9fcae1a

Merge branch 'main' into dsc_configure_module

6e3eb15

# Conflicts: # PowerToys.sln

f: implement docs/samples generator

Loading
Loading status checks…

64f9fe0

[ci]Sign FancyZonesEditorCommon.dll

Loading
Loading status checks…

b1e2286

jaimecbernardo added the merge-for-0.80 label Mar 25, 2024

Sign DSC files in the Generated folder

Loading
Loading status checks…

3b61840

yuyoyuppe added 4 commits March 28, 2024 19:19

f: address review comments

Loading
Loading status checks…

a39158d

Merge branch 'main' into dsc_configure_module

da2ef06

f: update usable options

Loading
Loading status checks…

4e9b334

f: add autogenerated sample

Loading
Loading status checks…

6c00343

htcfreek reviewed Apr 1, 2024

View reviewed changes

jaimecbernardo suggested changes Apr 1, 2024

View reviewed changes

jaimecbernardo added 13 commits April 1, 2024 12:10

[Installer] Don't use same GUID for different components

a2f5e2f

[Installer]Don't remove folders shared by other modules

6da3d6c

Allow configuring PTRun MaximumNumberOfResults

eaee7e0

Remove all settings DSC sample. Just random data

54800c5

Allow configuring Hosts Run as Administrator

Loading
Loading status checks…

b0285d1

Revert "[Installer]Don't remove folders shared by other modules"

Loading
Loading status checks…

dea50c3

This reverts commit 6da3d6c.

Add all PTRun plugins and Global and keyboard to DSC sample

Loading
Loading status checks…

dd3f35f

Fix issues with context menu modules not disabling

Loading
Loading status checks…

940080b

Fix default enabled values when setting with DSC

Loading
Loading status checks…

b6f0472

Fix tests regarding default modules in Settings

Loading
Loading status checks…

5d6eeaf

Merge branch 'main' into pr30918

Loading
Loading status checks…

c91d519

Fix merge error

Loading
Loading status checks…

f25955f

Restart PowerToys process if we stopped it

Loading
Loading status checks…

b7fad12

jaimecbernardo approved these changes Apr 1, 2024

View reviewed changes

jaimecbernardo merged commit f23fa3f into microsoft:main Apr 1, 2024
10 checks passed

jaimecbernardo mentioned this pull request Apr 8, 2024

PowerToys DSC Resource #28082

Closed

[DSC] Implement Microsoft.PowerToys.Configure DSCResource & winget support #30918

[DSC] Implement Microsoft.PowerToys.Configure DSCResource & winget support #30918

Conversation

yuyoyuppe commented Jan 12, 2024 • edited by jaimecbernardo Loading

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

htcfreek commented Feb 7, 2024

yuyoyuppe commented Feb 7, 2024

This comment has been minimized.

yuyoyuppe commented Mar 20, 2024

jaimecbernardo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaimecbernardo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaimecbernardo left a comment

Choose a reason for hiding this comment

htcfreek commented Apr 2, 2024

yuyoyuppe commented Jan 12, 2024 •

edited by jaimecbernardo

Loading