GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Change Get-Help cmdlet -Parameter parameter so it accepts string arrays. by sethvs · Pull Request #8454 · PowerShell/PowerShell

Conversation

Sergey Vasin (sethvs) commented Dec 12, 2018

Copy link

Copy Markdown

Contributor

PR Summary

Fix #8453

This PR changes Get-Help cmdlet -Parameter parameter so it accepts string arrays.
Also changes help function.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- User-facing Documentation needed
  - Issue filed - Issue link:
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
  - Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Get-Help cmdlet -Parameter parameter accepts string array.

7875ad4

Sergey Vasin (sethvs) requested review from Bruce Payette (BrucePay), Francisco Gamino (Francisco-Gamino), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) as code owners

December 12, 2018 12:20


          Fix variable names.

c36ba83

Ilya (iSazonov) reviewed

Dec 12, 2018

View reviewed changes

Ilya (iSazonov) left a comment

Copy link

Copy Markdown

Collaborator

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.

We need PowerShell Committee approval to change the public API.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/help/HelpCommands.cs Outdated

		get => _parameters;
		set => _parameters = value;
		}
		private string[] _parameters;

Ilya (iSazonov) Dec 12, 2018

Copy link

Copy Markdown

Collaborator

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.

Why we use the private variable? Why not public string[] Parameter { set; get; }

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergey Vasin (sethvs) Dec 13, 2018

Copy link

Copy Markdown

Contributor Author

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.

Agree.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label

Dec 12, 2018


          Use public string[] Parameter

592ab62

Aditya Patwardhan (adityapatwardhan) self-assigned this

Jan 7, 2019

Aditya Patwardhan (adityapatwardhan) requested changes

Jan 8, 2019

View reviewed changes

Comment thread

src/System.Management.Automation/help/HelpCommands.cs Outdated

		PSObject[] pInfos = helpInfo.GetParameter(Parameter);
		List<PSObject> pInfosList = new List<PSObject>(Parameter.Length);

		foreach (var parameter in Parameter)

Aditya Patwardhan (adityapatwardhan) Jan 8, 2019

Copy link

Copy Markdown

Member

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.

This block of code seems very similar to code at line # 443. Can you refactor it into a method?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergey Vasin (sethvs) Jan 9, 2019

Copy link

Copy Markdown

Contributor Author

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.

Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) commented Jan 9, 2019

Copy link

Copy Markdown

Collaborator

Sergey Vasin (@sethvs) Please fix style issues (excluding complex method).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergey Vasin (sethvs) commented Jan 9, 2019

Copy link

Copy Markdown

Contributor Author

Ilya (@iSazonov) For some reason CodeFactor still shows lines with issues from the first commit. There are no such lines now.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergey Vasin (sethvs) added 2 commits

January 9, 2019 08:21


          Refactor duplicate pieces of code into a method.

61d78cd


          Fix CodeFactor issues.

e15f6ea

Ilya (iSazonov) reviewed

Jan 9, 2019

View reviewed changes

Comment thread

src/System.Management.Automation/help/HelpCommands.cs

		if ((cat & supportedCategories) == 0)
		{
		if (!string.IsNullOrEmpty(Parameter))
		if (Parameter != null)

Ilya (iSazonov) Jan 9, 2019

Copy link

Copy Markdown

Collaborator

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.

Could you please comment why we need the change?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergey Vasin (sethvs) Jan 9, 2019

Copy link

Copy Markdown

Contributor Author

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.

Parameter is a string array now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Steve Lee (SteveL-MSFT) commented Jan 9, 2019

Copy link

Copy Markdown

Member

@PowerShell/powershell-committee reviewed this and is ok with the change

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels

Jan 9, 2019

Ilya (iSazonov) reviewed

Jan 10, 2019

View reviewed changes

Comment thread

src/System.Management.Automation/help/HelpCommands.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

src/System.Management.Automation/help/HelpCommands.cs

		/// <returns>Array of parameter infos.</returns>
		private PSObject[] GetParameterInfo(HelpInfo helpInfo)
		{
		List<PSObject> parameterInfosList = new List<PSObject>(Parameter.Length);

Ilya (iSazonov) Jan 10, 2019

Copy link

Copy Markdown

Collaborator

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.

We should consider moving Parameter != null checks to the method.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Sergey Vasin (sethvs) Jan 10, 2019

Copy link

Copy Markdown

Contributor Author

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'm not sure that this is necessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Update src/System.Management.Automation/help/HelpCommands.cs

2da2ab3

Co-Authored-By: sethvs <[email protected]>

Aditya Patwardhan (adityapatwardhan) commented Jan 14, 2019

Copy link

Copy Markdown

Member

Sergey Vasin (@sethvs) Can you please add tests for the new functionality?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergey Vasin (sethvs) added 5 commits

January 15, 2019 11:13


          Add test.


          Merge branch 'master' into helpParameter

49740db


          Add tag.

ed8d620


          Fix test.

423e50f


          Testing.

4bafe7f

Sergey Vasin (sethvs) commented Jan 15, 2019 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

Error (no failed tests) with macOS check.
Close and reopen to restart.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergey Vasin (sethvs) closed this

Jan 15, 2019

Sergey Vasin (sethvs) reopened this

Jan 15, 2019

Sergey Vasin (sethvs) commented Jan 15, 2019

Copy link

Copy Markdown

Contributor Author

macOS check throws Dependency precheck failed!
Again, no failed tests.
Guys, can you take a look into it?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sergey Vasin (sethvs) mentioned this pull request

Jan 15, 2019

Fix a typo in packaging.psm1 #8647

Merged

7 of 11 tasks

Aditya Patwardhan (adityapatwardhan) commented Jan 15, 2019

Copy link

Copy Markdown

Member

Having a look.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) commented Jan 15, 2019

Copy link

Copy Markdown

Member

It seems that Bootstrap is failing with the following issue: Travis Plunk (@TravisEz13) any idea?

2019-01-15T17:51:59.5248120Z Warning: openssl 1.0.2q is already installed and up-to-date
2019-01-15T17:51:59.5248480Z To reinstall 1.0.2q, run `brew reinstall openssl`
2019-01-15T17:51:59.5424690Z Error: cmake 3.13.2 is already installed
2019-01-15T17:51:59.5424850Z To upgrade to 3.13.3, run `brew upgrade cmake`
2019-01-15T17:52:01.9032320Z Warning: curl 7.63.0 is already installed and up-to-date
2019-01-15T17:52:01.9032780Z To reinstall 7.63.0, run `brew reinstall curl`
2019-01-15T17:52:06.2156000Z Successfully installed cabin-0.9.0
2019-01-15T17:52:06.2156670Z Successfully installed backports-3.11.4
2019-01-15T17:52:06.2157410Z Successfully installed arr-pm-0.0.10
2019-01-15T17:52:06.2157950Z Successfully installed clamp-1.0.1
2019-01-15T17:52:06.2158020Z Building native extensions. This could take a while...
2019-01-15T17:53:06.0164980Z Before reporting this, could you check that the file you're documenting
2019-01-15T17:53:06.0165390Z has proper syntax:
2019-01-15T17:53:06.0165830Z 
2019-01-15T17:53:06.0166810Z   /usr/local/opt/ruby/bin/ruby -c lib/childprocess/tools/generator.rb
2019-01-15T17:53:06.0167080Z 
2019-01-15T17:53:06.0167210Z RDoc is not a full Ruby parser and will fail when fed invalid ruby programs.
2019-01-15T17:53:06.0167280Z 
2019-01-15T17:53:06.0167360Z The internal error was:
2019-01-15T17:53:06.0167540Z 
2019-01-15T17:53:06.0168380Z 	(NoMethodError) undefined method `[]' for nil:NilClass
2019-01-15T17:53:06.0168460Z 
2019-01-15T17:53:06.0168880Z ERROR:  While executing gem ... (NoMethodError)
2019-01-15T17:53:06.0169910Z     undefined method `[]' for nil:NilClass
2019-01-15T17:53:06.0245480Z Successfully installed ffi-1.10.0

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) approved these changes

Jan 15, 2019

View reviewed changes

Aditya Patwardhan (adityapatwardhan) merged commit 279993b into PowerShell:master

Jan 16, 2019

Aditya Patwardhan (adityapatwardhan) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label

Jan 16, 2019

Sergey Vasin (sethvs) deleted the helpParameter branch

January 16, 2019 08:09

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Change Get-Help cmdlet -Parameter parameter so it accepts string arra…

95ae8d3

…ys. (PowerShell#8454)

Reviewers

Ilya (iSazonov)

iSazonov left review comments

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Bruce Payette (BrucePay)

Awaiting requested review from BrucePay

Dongbo Wang (daxian-dbw)

Awaiting requested review from daxian-dbw

daxian-dbw is a code owner

Francisco Gamino (Francisco-Gamino)

Awaiting requested review from Francisco-Gamino

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Aditya Patwardhan (adityapatwardhan)

Labels

CL-General

Indicates that a PR should be marked as a general cmdlet change in the Change Log

Committee-Reviewed

PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow Get-Help cmdlet -Parameter parameter to accept string array.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Sergey Vasin (sethvs) commented Dec 12, 2018

PR Summary

PR Checklist

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Jan 9, 2019

Uh oh!

Sergey Vasin (sethvs) commented Jan 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Steve Lee (SteveL-MSFT) commented Jan 9, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Jan 14, 2019

Uh oh!

Sergey Vasin (sethvs) commented Jan 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sergey Vasin (sethvs) commented Jan 15, 2019

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Jan 15, 2019

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Jan 15, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Sergey Vasin (sethvs) commented Jan 15, 2019 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.