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

refactor to splat #2

Merged
merged 1 commit into from Jun 30, 2019

Conversation

Projects
None yet
3 participants
@TylerLeonhardt
Copy link
Contributor

commented Jun 28, 2019

Splatting is a very PowerShell-y concept. The idea is that you define the parameters for your command in a hashtable and then use the @ sign instead of the $ to "splat" them onto a command.

This change refactors to that so we don't have to use Add-Member.

@TylerLeonhardt TylerLeonhardt force-pushed the TylerLeonhardt:refactor-to-splat branch from 5dc3117 to 8b8f101 Jun 28, 2019

@shanselman

This comment has been minimized.

Copy link
Owner

commented Jun 28, 2019

Sorry I was messing around. Would you rebase on top of the new stuff?

@TylerLeonhardt TylerLeonhardt force-pushed the TylerLeonhardt:refactor-to-splat branch from 8b8f101 to 71d0c97 Jun 29, 2019

@TylerLeonhardt

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

Done!

@gpduck

This comment has been minimized.

Copy link

commented Jun 29, 2019

Another problem with the existing if statements is they won't for values that cast to $false (like empty string, $null, and 0):

if(!$terminalProfile.backgroundImage){
    $terminalProfile | Add-Member -NotePropertyName backgroundImage -NotePropertyValue ""
}

If backgroundImage is present but set to "", PowerShell will cast the "" to $false and then try to add the member on top of the existing member, generating an error. A safe way to implement the same logic would be to use Get-Member to test for the property existance:

if(!(Get-Member -Name backgroundImage -InputObject $TerminalProfile)) {
    $terminalProfile | Add-Member -NotePropertyName backgroundImage -NotePropertyValue ""
}

But switching to use the parameters on Set-MSTerminalProfile is a better solution :)

@shanselman

This comment has been minimized.

Copy link
Owner

commented Jun 29, 2019

So sorry about this, but you still stomped on the Seconds parameter and set it back to a hard coded 5?

@TylerLeonhardt

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2019

Ah. No worries. I'll fix 👍

@TylerLeonhardt TylerLeonhardt force-pushed the TylerLeonhardt:refactor-to-splat branch from 71d0c97 to 4bcd5b4 Jun 30, 2019

@TylerLeonhardt

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

Ok there we go! I even moved over the Mood script - only 28 lines (most of which are parameter handling & whitespace) not bad!

@shanselman shanselman merged commit d0e98a0 into shanselman:Main Jun 30, 2019

@shanselman

This comment has been minimized.

Copy link
Owner

commented Jun 30, 2019

So clean @TylerLeonhardt! :shipit:

@TylerLeonhardt TylerLeonhardt deleted the TylerLeonhardt:refactor-to-splat branch Jun 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.