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

Set-WinGetUserSettings needs distinct parameters #4536

Open
jdhitsolutions opened this issue Jun 4, 2024 · 11 comments
Open

Set-WinGetUserSettings needs distinct parameters #4536

jdhitsolutions opened this issue Jun 4, 2024 · 11 comments
Labels
Issue-Feature This is a feature request for the Windows Package Manager client. PowerShell Issue related to WinGet PowerShell Module or cmdlet

Comments

@jdhitsolutions
Copy link

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.

@jdhitsolutions jdhitsolutions added the Issue-Feature This is a feature request for the Windows Package Manager client. label Jun 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage Issue need to be triaged label Jun 4, 2024
@denelon denelon removed the Needs-Triage Issue need to be triaged label Jun 4, 2024
@denelon
Copy link
Contributor

denelon commented Jun 4, 2024

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?

@denelon denelon added the PowerShell Issue related to WinGet PowerShell Module or cmdlet label Jun 4, 2024
@jdhitsolutions
Copy link
Author

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
Copy link

stephengillie commented Jun 4, 2024

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
Copy link
Author

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

@denelon
Copy link
Contributor

denelon commented Jun 4, 2024

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
Copy link
Author

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

@denelon
Copy link
Contributor

denelon commented Jun 12, 2024

@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
Copy link
Contributor

denelon commented Jun 12, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

denelon commented Jun 12, 2024

Makes sense. Thanks for the examples!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client. PowerShell Issue related to WinGet PowerShell Module or cmdlet
Projects
None yet
Development

No branches or pull requests

3 participants