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

Redirecting to a variable should be possible #20381

Merged
merged 17 commits into from
Jun 11, 2024

Conversation

JamesWTruher
Copy link
Member

@JamesWTruher JamesWTruher commented Sep 27, 2023

PR Summary

There are a number of situations where it would be very convenient to redirect data to a variable using the variable:name syntax. This PR provides this behavior by inspecting the target of the redirection and if it uses the variable provider will call Set-Variable rather than Out-File.

With this PR the following scenarios are possible:

PS /> .{                                                                                   
>> "Output 1"
>> Write-Warning "Warning, Warning!"
>> "Output 2"
>> } 3> variable:warns
Output 1
Output 2
PS /> $warns
WARNING: Warning, Warning!
PS /> 

This could prove very helpful to some logging scenarios as well.

S /> .{                                                                                   
>> "Output 1"
>> Write-Warning "This is a warning"
>> "Output 2"
>> Write-Debug -Debug "and a debug message"
>> "Output 3"
>> } *> variable:kitchenSync
PS /> $kitchenSync
Output 1
WARNING: This is a warning
Output 2
DEBUG: and a debug message
Output 3

Append is also supported via >>

PS /> .{ "output 1" } *>variable:outputs                                                   
PS /> .{ "output 2" } *>>variable:outputs
PS /> .{ write-warning warn! } *>>variable:outputs
PS /> .{ "output 3" } *>>variable:outputs         
PS /> $outputs                   
output 1
output 2
WARNING: warn!
output 3

and redirecting multiple streams to multiple targets, based on the stream number:

PS /> .{
>> "output 1"
>> write-warning "Warning!"
>> "output 2"
>> write-debug -debug "debug here"
>> "output 3"
>> write-information Info
>> "output 4"
>> write-error "That's bad!"
>> Write-Verbose -Verbose "and verbose"
>> "all done"
>> } > variable:outputStream 2>variable:errorStream 3>variable:warningStream 4>variable:verboseStream 5>variable:debugStream 6>variable:InfoStream
PS /> $outputStream
output 1
output 2
output 3
output 4
all done
PS /> $errorStream 
Write-Error: That's bad!
PS /> $warningStream
WARNING: Warning!
PS /> $verboseStream                   
VERBOSE: and verbose
PS /> $debugStream  
DEBUG: debug here
PS /> $InfoStream 
Info

Lastly, object-ness is preserved:

PS /> .{                                 
>> "output"
>> write-error "bad"
>> write-warning "warn"
>> write-verbose -verbose "verbose"
>> write-debug -debug debug
>> } *>variable:all
PS /> $all | %{$_.gettype()}

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     True     String                                   System.Object
True     False    ErrorRecord                              System.Object
True     False    WarningRecord                            System.Management.Automation.InformationalRecord
True     False    VerboseRecord                            System.Management.Automation.InformationalRecord
True     False    DebugRecord                              System.Management.Automation.InformationalRecord

This is not to say that redirection to a variable should take the place of assignment, but it does enable some new scenarios.

the current behavior when redirecting to a variable is an error (because redirection is implemented via Out-File):

PS /> "sldfkj" >variable:no          
Out-File: Cannot open file because the current provider (Microsoft.PowerShell.Core\Variable) cannot open a file.

PR Context

PR Checklist

@JamesWTruher JamesWTruher added Issue-Enhancement the issue is more of a feature request than a bug WG-Engine core PowerShell engine, interpreter, and runtime labels Sep 27, 2023
@JamesWTruher JamesWTruher changed the title Redirecting to a variable should be possible WIP: Redirecting to a variable should be possible Sep 28, 2023
@JamesWTruher JamesWTruher changed the title WIP: Redirecting to a variable should be possible Redirecting to a variable should be possible Sep 28, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Oct 5, 2023
@StevenBucher98 StevenBucher98 added the PowerShell-Docs needed The PR was reviewed and a PowerShell Docs update is needed label Oct 9, 2023
@SeeminglyScience SeeminglyScience added the WG-Reviewed A Working Group has reviewed this and made a recommendation label Nov 13, 2023
Copy link
Collaborator

@SeeminglyScience SeeminglyScience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some very minor style nits, other than that looks great! Very excited for this change ❤️

This PR has 183 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Medium
Size       : +173 -10
Percentile : 56.6%

Total files changed: 7

Change summary by file extension:
.cs : +56 -9
.resx : +3 -0
.ps1 : +114 -1

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

This PR has 183 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Medium
Size       : +173 -10
Percentile : 56.6%

Total files changed: 7

Change summary by file extension:
.cs : +56 -9
.resx : +3 -0
.ps1 : +114 -1

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

This PR has 186 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Medium
Size       : +174 -12
Percentile : 57.2%

Total files changed: 7

Change summary by file extension:
.cs : +57 -11
.resx : +3 -0
.ps1 : +114 -1

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@JamesWTruher JamesWTruher added the KeepOpen The bot will ignore these and not auto-close label Apr 16, 2024
Update test to check for experimental feature status.
Set-Variable will still have the -append parameter, but if used when the experimental feature is disabled, a parameter binding error will result.
JamesWTruher and others added 5 commits April 16, 2024 12:13
Update Set-Variable -Append to be correct when -name and -value are used.
Add tests for the new behavior in Set-Variable.
Add tests to validate.
// Now, We can take what ever has been set if PSDefaultParameterValues
// Unicode is still the default, but now may be overridden
// determine whether we're trying to set a variable by inspecting the file path
// if we can determine that it's a variable, we'll use Set-Variable rather than Out-File
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note and not something you need to change, but it seems in the future, it would make more sense to have Out-Provider and have the provider subsystem determine which provider receives it rather than hardcoding to specific cases here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did think about that when I put this together and thought that the scenarios for redirection to something other than filesystem or variable were pretty limited, so I took this approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that a smaller change now makes sense. Created #23794 to capture this.

@microsoft-github-policy-service microsoft-github-policy-service bot added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Review - Needed The PR is being reviewed labels May 13, 2024
JamesWTruher and others added 3 commits May 13, 2024 10:03
…cOps.cs

Co-authored-by: Steve Lee <slee@microsoft.com>
…cOps.cs

Co-authored-by: Steve Lee <slee@microsoft.com>
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label May 13, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label May 21, 2024
@SeeminglyScience SeeminglyScience merged commit 167a492 into PowerShell:master Jun 11, 2024
37 checks passed
@SeeminglyScience SeeminglyScience removed their assignment Jun 11, 2024
Copy link
Contributor

microsoft-github-policy-service bot commented Jun 11, 2024

📣 Hey @JamesWTruher, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@SeeminglyScience SeeminglyScience added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed Review - Needed The PR is being reviewed KeepOpen The bot will ignore these and not auto-close labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Engine Indicates that a PR should be marked as an engine change in the Change Log Issue-Enhancement the issue is more of a feature request than a bug Medium PowerShell-Docs needed The PR was reviewed and a PowerShell Docs update is needed WG-Engine core PowerShell engine, interpreter, and runtime WG-Reviewed A Working Group has reviewed this and made a recommendation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants