-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Add tooltips for hashtable key completions #17864
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
Conversation
Add completion for calculated properties for Compare-Object and ConvertTo-HTML Add argument completer for the Property parameter of ConvertTo-Html Use resource files instead of dictionary for comment completion tooltips
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
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
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
@daxian-dbw Can you review? |
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
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) |
@MartinGC94 Are all these changes related, or can you break them down into different PRs? |
@iSazonov Everything except the additional switch cases for -Edit: I changed my mind and removed the irrelevant changes. The PR could be further split into tooltips for hashtables, and updating the tooltips for help keywords to use the resource file but then I'd have to deal with annoying merge conflicts so I'd prefer not to. |
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
</data> | ||
<data name="DepthHashtableKeyDescription" xml:space="preserve"> | ||
<value>[int] | ||
The depth key allows you to specify the depth of expansion per property.</value> |
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.
Please reword like:
The depth key allows you to specify the depth of expansion per property.</value> | |
The depth key specifies the depth of expansion per property.</value> |
</data> | ||
<data name="AscendingHashtableKeyDescription" xml:space="preserve"> | ||
<value>[bool] | ||
Allows you to specify the order of sorting for one or more properties.</value> |
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.
The same.
</data> | ||
<data name="DescendingHashtableKeyDescription" xml:space="preserve"> | ||
<value>[bool] | ||
Allows you to specify the order of sorting for one or more properties.</value> |
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.
The same.
</data> | ||
<data name="DataHashtableKeyDescription" xml:space="preserve"> | ||
<value>[string[]] | ||
Selects events with any of the specified values in the EventData section.</value> |
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.
Can we add an example in the help?
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.
The tooltips generally don't have examples. I'm also not sure how much value there would be in something like: Get-WinEvent -FilterHashtable @{LogName="System";ID=7040;Data="BITS"}
because unless you happen to have the raw event data in front of you, you won't know what field "BITS" refers to. The same goes for the SuppressHashFilter
suggestion.
</data> | ||
<data name="SuppressHashFilterHashtableKeyDescription" xml:space="preserve"> | ||
<value>[hashtable] | ||
Excludes events that match the values specified in the hashtable.</value> |
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.
The same.
<data name="CommentHelpSYNOPSISKeywordDescription" xml:space="preserve"> | ||
<value>A brief description of the function or script. | ||
This keyword can be used only once in each topic.</value> | ||
</data> | ||
<data name="CommentHelpDESCRIPTIONKeywordDescription" xml:space="preserve"> | ||
<value>A detailed description of the function or script. | ||
This keyword can be used only once in each topic.</value> | ||
</data> | ||
<data name="CommentHelpPARAMETERKeywordDescription" xml:space="preserve"> | ||
<value>.PARAMETER <Parameter-Name> | ||
The description of a parameter. | ||
Add a .PARAMETER keyword for each parameter in the function or script syntax.</value> | ||
</data> |
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.
Maybe use the same pattern started with keyword everywhere?
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.
Do you mean replacing all the *HashtableKeyDescription
instances with HashtableKeyDescription*
? (Example: StartTimeHashtableKeyDescription -> HashtableKeyDescriptionStartTime)
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.
I mean .PARAMETER
in help text.
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.
I see. The point of .PARAMETER <Parameter-Name>
is to show the required syntax. I use it for all the help keywords that take some sort of parameter, for example: EXTERNALHELP
and FORWARDHELPCATEGORY
. For the help keywords that don't take any arguments (SYNOPSIS
and others) I don't see any reason to include an extra line with just the keyword.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
📣 Hey @MartinGC94, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Fixes #15450
Add tooltips for hashtable key completions
Use resource files instead of dictionary for comment completion tooltips
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).