microsoft / winget-cli

WinGet is the Windows Package Manager. This project includes a CLI (Command Line Interface), PowerShell modules, and a COM (Component Object Model) API (Application Programming Interface).
https://learn.microsoft.com/windows/package-manager/
MIT License
22.5k stars 1.39k forks source link

Set-WinGetUserSettings needs distinct parameters #4536

Open jdhitsolutions opened 3 weeks ago

jdhitsolutions commented 3 weeks ago

Description of the new feature / enhancement

The Set-WinGetUserSettings cmdlets takes a hashtable of user settings. I think it would follow better PowerShell practice to define each setting as a separate parameter. If the user has a hashtable, they can splat to the cmdlet. The current design puts a burden on the user when they only want to modify a single setting.

Proposed technical implementation details

Add parameters for settings like ProgressBar and UpdateInterval. I would even suggest creating a separate command to set experimental features.

denelon commented 3 weeks ago

We went back and forth on the design choice with that. I can see the value in both approaches. We're also going to be doing some work with DSC v3 to enable WinGet to act as its own DSC resource.

Given the distinction that some settings are user based and some require elevation, would you include that in the name, or just as a part of the output when the cmdlet is called? Naming things is hard...

What names and arguments would you suggest?

jdhitsolutions commented 3 weeks ago

You should make this as easy for the user as possible. Don't force them to know the technical details. Give them an easy to use parameter that accepts pipeline input by property name. They can then pass parameters values directly, splat, or pipe into the command.

stephengillie commented 3 weeks ago

~Requiring a properly-formatted hashtable of data is an advanced user input method. PowerShell functions and cmdlets should be open to all users, not just advanced users who have researched a particular area.~

~The way to do this is to construct the PowerShell function in such a way that the upper parameters define the details - and the last parameter accept the hashtable, with its default value being a hashtable of all of the parameters above it. This is essentially a "sideways wrapper" with an advanced input method.~

~Example:~

Function Get-MonthAndDay {
    param(
        $Month = 7,
        $Day = 4,
        $hash = @{ month = $month; day = $day }
    )
    Get-Date -Month $hash.month -Day $hash.day
}
jdhitsolutions commented 3 weeks ago

You don't need a parameter that accepts a hashtable. That's what splatting is for.

denelon commented 3 weeks ago

This is something I use for DSC Resource testing:

$SplatParam = @{
    Name       = 'WindowsOptionalFeature'
    ModuleName = 'PSDesiredStateConfiguration'
    Method     = 'Test'
    Property   = @{
        Name      = 'Containers-DisposableClientVM'
        Ensure    = 'Enable'
    }
}

Invoke-DscResource @SplatParam
$SplatParam = @{
    Name       = 'WindowsOptionalFeature'
    ModuleName = 'PSDscResources'
    Method     = 'Test'
    Property   = @{
        Name      = 'Containers-DisposableClientVM'
        Ensure    = 'Present'
    }
}

Invoke-DscResource @SplatParam

I think what is being suggested is to be able to do something like this for the WinGet settings.

jdhitsolutions commented 3 weeks ago

Don't commingle the design considerations of a DSC resource with a PowerShell commands. They are very distinct in their requirements.

denelon commented 2 weeks ago

@jdhitsolutions, what kind of proposed example do you have here? I value your expertise over mine very highly here 😊

I was just showing an example of what I have seen in terms of splatting. I could be way off base. I'm just being transparent about my ignorance.

denelon commented 2 weeks ago

I do see the distinction between the interface we're proposing for WinGet as a DSC v3 resource and the PowerShell cmdlets a user would want to use to manage settings.

jdhitsolutions commented 2 weeks ago

I'd approach this in a more granular approach. First, how likely is the user going to need to change the Schema value? I also thing the default behavior is always to merge. Only set what the user specifies. I think all you need is a command with syntax like this:

PS D:\> Get-Command Set-WinGetUserSetting -syntax 

Set-WinGetUserSetting [[-AutoUpdateInterval] <int>] [[-ProgressBar] <string>] [<CommonParameters>]

I'd create a separate command called Set-WingetUserExperimentalFeature that right now would look like :

PS D:\> Get-Command Set-WinGetUserExperimentalSetting -syntax

Set-WinGetUserExperimentalSetting [-List] [-Upgrade] [<CommonParameters>]

As you add experimental features you would need to update this command. Also note that I changed to a singular noun to meet the accepted paradigm. From a DSC perspective, I can see where I want to configure a group of settings, but as an end-user I am likely to only need to change a single setting, like the progress bar. Does this make sense?

jdhitsolutions commented 2 weeks ago

Actually, I'd have to think a bit further on the experimental features cmdlet because I would need a way to turn something off and I don't like parameters that take Boolean values. But hopefully, you still get the idea.

denelon commented 2 weeks ago

Makes sense. Thanks for the examples!