Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add completion single/double quote support for-Noun parameter#24977

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

ArmaanMcleod
Copy link
Contributor

@ArmaanMcleodArmaanMcleod commentedFeb 9, 2025
edited
Loading

PR Summary

UseCompletionCompleters.GetMatchingResults inNounArgumentCompleter so-Noun parameter forGet-Command has support for single/double quotes.

Also made the following improvements:

  • When-Verb is used, only get nouns which are possible with the following verbs.
  • useSortedSet<string> withStringComparer.IgnoreCase to ensure all nouns are unique and sorted and case insensitive. Also removed previous LINQOrder() call with this change.
  • WrapGet-Command logic intoCompletionCompleters.GetCommandInfo method. This can probably be used elsewhere and potentially reduce duplication of command querying code in completers. Also addusing for creating powershell instance so its automatically disposed.

I also added some tab completion tests which seem to be missing for this completer.

PR Context

Related to#24873

PR Checklist

@iSazonov
Copy link
Collaborator

/azp run

@azure-pipelinesAzure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@iSazonov
Copy link
Collaborator

/azp run

@azure-pipelinesAzure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@iSazonov
Copy link
Collaborator

@ArmaanMcleod Please look failed tests. It seems previously had fake ordering for CompletionResult type.

@ArmaanMcleod
Copy link
ContributorAuthor

@iSazonov It looks like tests need to exclude Gridview cmdlets for Mac and Linux, hench test failures. These cmdlets only available on Windows.

I will fix this.

iSazonov reacted with thumbs up emoji

@iSazonov
Copy link
Collaborator

/azp run

@azure-pipelinesAzure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@ArmaanMcleod
Copy link
ContributorAuthor

@iSazonov I think it should be fine now. Thanks for the help.

I ran tests on Windows and Linux and it seems to be fine, hoping CI lets it go through 😄.

@@ -1724,7 +1724,7 @@ public IEnumerable<CompletionResult> CompleteArgument(
private static SortedSet<string> GetCommandNouns(IDictionary fakeBoundParameters)
{
Collection<CommandInfo> commands = CompletionCompleters.GetCommandInfo(fakeBoundParameters, "Module", "Verb");
SortedSet<string> nouns = new(StringComparer.OrdinalIgnoreCase);
SortedSet<string> nouns = new();
Copy link
Collaborator

@iSazonoviSazonovFeb 10, 2025
edited
Loading

Choose a reason for hiding this comment

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

If cmdlet names are case-insensetive on all OS-s this must be case-insensetive too. I think we should think about fixing tests only.

Copy link
ContributorAuthor

@ArmaanMcleodArmaanMcleodFeb 10, 2025
edited
Loading

Choose a reason for hiding this comment

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

Yep I agree, although to completer it does not make a difference. I have updated code & test. Only way is to use sorted set in the test becauseSort-Object -Unique gives different ordering on Linux. I think this is fine because it is an accurate comparison anyways.

@iSazonov

This comment was marked as outdated.

@azure-pipelinesAzure Pipelines

This comment was marked as outdated.

@iSazonoviSazonov added the CL-GeneralIndicates that a PR should be marked as a general cmdlet change in the Change Log labelFeb 11, 2025
@iSazonoviSazonov self-assigned thisFeb 11, 2025
@iSazonoviSazonov merged commit0f7ba43 intoPowerShell:masterFeb 11, 2025
39 of 41 checks passed
@microsoft-github-policy-serviceMicrosoft GitHub Policy Service
Copy link
Contributor

microsoft-github-policy-servicebot commentedFeb 11, 2025
edited by unfurl-linksbot
Loading

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

🔗https://aka.ms/PSRepoFeedback

@daxian-dbwdaxian-dbw mentioned this pull requestFeb 21, 2025
21 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@iSazonoviSazonoviSazonov approved these changes

Assignees

@iSazonoviSazonov

Labels
CL-GeneralIndicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ArmaanMcleod@iSazonov

[8]ページ先頭

©2009-2025 Movatter.jp