-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
Improve enumeration of inferred types in pipeline #17799
Improve enumeration of inferred types in pipeline #17799
Conversation
Co-authored-by: Ilya <darpa@yandex.ru>
@MartinGC94 Please look CI fails. |
@iSazonov Sorry, one more minor change so this works as expected:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed off as maintainer
@daxian-dbw - I would like your review as a SME. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except that the parameter name should use camel case.
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Dongbo Wang <dongbow@microsoft.com>
@MartinGC94 You will need to do additional changes on the uses of that parameter within different methods. My suggested changes above didn't include those. |
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
🎉 Handy links: |
PR Summary
Update the type inference for $_ so it uses the same enumeration as the foreach loop.
Update the native member completion for the *-Object commands to enumerate the types of the input object.
Update the member completion for foreach-object to not add the parenthesis when completing methods.
Update the type inference for Where, Sort and Select so the inferred type after these commands are enumerated.
PR Context
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).