Skip to content

Add -PropertyType argument completer for New-ItemProperty #21117

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Dec 25, 2024
Merged

Add -PropertyType argument completer for New-ItemProperty #21117

merged 20 commits into from
Dec 25, 2024

Conversation

ArmaanMcleod
Copy link
Contributor

@ArmaanMcleod ArmaanMcleod commented Jan 20, 2024 •
edited
Loading

PR Summary

Fixes #21116

This PR adds an argument completer for Option 1 described in linked issue.

Add -PropertyType argument completer for New-ItemProperty command.

PR Context

Currently you cannot tab complete registry value kinds for New-ItemProperty -PropertyType. This PR adds a PropertyTypeArgumentCompleter which matches wordToComplete against the possible enum values of RegistryValueKind.

PR Checklist

@ArmaanMcleod ArmaanMcleod changed the title Add -PropertyType argument completer for New-ItemProperty Option 1 - Add -PropertyType argument completer for New-ItemProperty Jan 20, 2024
@ArmaanMcleod ArmaanMcleod mentioned this pull request Jan 20, 2024
Closed
22 tasks
@ArmaanMcleod ArmaanMcleod changed the title Option 1 - Add -PropertyType argument completer for New-ItemProperty WIP: Option 1 - Add -PropertyType argument completer for New-ItemProperty Jan 21, 2024
@ArmaanMcleod ArmaanMcleod marked this pull request as draft January 21, 2024 11:18
@ArmaanMcleod ArmaanMcleod marked this pull request as ready for review January 21, 2024 13:17
@ArmaanMcleod ArmaanMcleod requested a review from iSazonov January 21, 2024 13:17
@ArmaanMcleod
Copy link
Contributor Author

Thanks @iSazonov, I've changed implementation to check if registry provider is used.

Also @MartinGC94 let me know what you think, took some inspiration from your argument completer 🙂.

@ArmaanMcleod ArmaanMcleod changed the title WIP: Option 1 - Add -PropertyType argument completer for New-ItemProperty Option 1 - Add -PropertyType argument completer for New-ItemProperty Jan 21, 2024
MartinGC94
MartinGC94 suggested changes Jan 21, 2024
View reviewed changes
Copy link
Contributor

@MartinGC94 MartinGC94 left a comment

Choose a reason for hiding this comment

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

Overall logic seems fine, but I agree with @iSazonov suggestion about not converting all of the paths to an array. If you change that + my minor suggestions then I would be fine with this.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jan 21, 2024
@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 Jan 22, 2024
@ArmaanMcleod
Copy link
Contributor Author

Thanks @iSazonov and @MartinGC94 for the comprehensive and helpful feedback. I have made changes to address the comments, please have a look and let me know if I've missed anything.

For the multiple paths, I am now just running Resolve-Path on the fakeBoundParameters as is and checking the first path, got rid of the array converting.

@ArmaanMcleod ArmaanMcleod force-pushed the new-itemproperty-property-type-completer branch from 7763f77 to d0373c0 Compare January 22, 2024 09:42
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Jan 30, 2024
@ArmaanMcleod ArmaanMcleod changed the title Option 1 - Add -PropertyType argument completer for New-ItemProperty Add -PropertyType argument completer for New-ItemProperty Jul 7, 2024
@ArmaanMcleod @iSazonov
Added argument completer for PropertyType parameter
08f7c00
@ArmaanMcleod @iSazonov
Added tests
e462c35
@ArmaanMcleod @iSazonov
Fix variables
69f303b
@ArmaanMcleod @iSazonov
Skip test if not windows and hardcode list of registry value kinds
25c6841
@ArmaanMcleod @iSazonov
Fixing codefactor
d5c0b82
@ArmaanMcleod @iSazonov
Add space between System directives sand Microsoft.Win32 directive
261ce56
@ArmaanMcleod @iSazonov
Use IsWindows automatic variable instead of fully qualified type
aed0aa4
@ArmaanMcleod @iSazonov
Change argument completer to check registry provider
9096e07
@ArmaanMcleod @iSazonov
Fix codefactor for XML period
ff6670a
@ArmaanMcleod @iSazonov
Remove all comments
01016d1
@ArmaanMcleod @iSazonov
Remove Microsoft.Win32 directive
8d8cc73
@ArmaanMcleod @iSazonov
Formatting
832048c
@ArmaanMcleod @iSazonov
Address feedback comments
727e458
@ArmaanMcleod @iSazonov
Skip extra test if not windows
619f36b
@ArmaanMcleod @iSazonov
Check paths count
9ea0eb8
@ArmaanMcleod @iSazonov
Wrap registry detection code in method
1b2f06d
@ArmaanMcleod @iSazonov
Fix code factor
e669b9f
@ArmaanMcleod @iSazonov
Remove windows platform check and instead wrap in #if !UNIX
c34be9b
@ArmaanMcleod @iSazonov
Add #if !UNIX around whole argument completer class
6213c6a
@ArmaanMcleod @iSazonov
Use internal HandleDoubleAndSingleQuote method to do quoting
a8d8dc3
@iSazonov iSazonov force-pushed the new-itemproperty-property-type-completer branch from 2af476e to a8d8dc3 Compare December 25, 2024 11:42
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Dec 25, 2024
@iSazonov iSazonov self-assigned this Dec 25, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 25, 2024
@iSazonov iSazonov merged commit e127345 into PowerShell:master Dec 25, 2024
38 checks passed
@microsoft-github-policy-service Microsoft GitHub Policy Service
Copy link
Contributor

microsoft-github-policy-service bot commented Dec 25, 2024 •
edited by unfurl-links bot
Loading

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

🔗 https://aka.ms/PSRepoFeedback

@ArmaanMcleod ArmaanMcleod deleted the new-itemproperty-property-type-completer branch December 27, 2024 04:34
@ArmaanMcleod ArmaanMcleod mentioned this pull request Jan 22, 2025
Merged
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iSazonov iSazonov iSazonov approved these changes

@MartinGC94 MartinGC94 MartinGC94 approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan

Assignees

@iSazonov iSazonov

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

Successfully merging this pull request may close these issues.

New-ItemProperty -PropertyType should have tab completion
3 participants
@ArmaanMcleod @iSazonov @MartinGC94