Skip to content

Add an Exclusion filter to the Command Explorer. - #1686

Merged
TylerLeonhardt merged 4 commits into
PowerShell:masterfrom
corbob:Filter-Command-Explorer
Jan 28, 2019
Merged

Add an Exclusion filter to the Command Explorer.#1686
TylerLeonhardt merged 4 commits into
PowerShell:masterfrom
corbob:Filter-Command-Explorer

Conversation

@corbob

@corbob Cory Knox (corbob) commented Jan 8, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Add an Exclusion filter to the Command Explorer.

Suggested in #1677

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • NA PR has tests
  • 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

TylerLeonhardt
TylerLeonhardt reviewed Jan 8, 2019
View reviewed changes

@TylerLeonhardt TylerLeonhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good 👍 a couple thoughts

Comment thread package.json Resolved
Comment thread src/features/GetCommands.ts Resolved
@corbob Cory Knox (corbob) changed the title WIP: Add an Exclusion filter to the Command Explorer. Add an Exclusion filter to the Command Explorer. Jan 17, 2019
@corbob

Cory Knox (corbob) commented Jan 17, 2019

Copy link
Copy Markdown
Contributor Author

TylerLeonhardt I believe I've covered off all the comments.

Please let me know if there is anything else needed for this PR.

TylerLeonhardt
TylerLeonhardt approved these changes Jan 18, 2019
View reviewed changes

@TylerLeonhardt TylerLeonhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 😊

@TylerLeonhardt

TylerLeonhardt commented Jan 18, 2019

Copy link
Copy Markdown
Member

Cory Knox (@corbob) do you think that result could ever be null here? That's my only concern.

@corbob

Cory Knox (corbob) commented Jan 18, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Cory Knox (@corbob) do you think that result could ever be null here? That's my only concern.

I don't believe it should ever happen, but I'll do some modifications of PSES to send back a null and make sure we handle that properly on the off chance null comes back.

Edit: No changes to PSES needed. I can just redefine Microsoft.PowerShell.Core\Get-Command: pwsh redefining Get-Command

With this revelation, I'll do some testing and make sure we build for nothing coming back from Get-Command.

@TylerLeonhardt

TylerLeonhardt commented Jan 19, 2019

Copy link
Copy Markdown
Member

cool cool. Looks good then!

@corbob

Cory Knox (corbob) commented Jan 20, 2019

Copy link
Copy Markdown
Contributor Author

So I did a test, and result comes back as an empty array when get-command just does a write-host:

image

Tested on both PowerShell 5.1 and 6.2.

@TylerLeonhardt
TylerLeonhardt merged commit 3589f92 into PowerShell:master Jan 28, 2019
TylerLeonhardt pushed a commit that referenced this pull request Feb 2, 2019
@corbob @TylerLeonhardt
Add an Exclusion filter to the Command Explorer. (#1686)
cc24c5a 
* Work with comma separated exclude filter

* Add a filter...

* Adjust filter. Remove case sensetivity.

* Change Exclude Fitler to array.
@n0spaces Matt Schwartz (n0spaces) mentioned this pull request Aug 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@TylerLeonhardt TylerLeonhardt TylerLeonhardt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@corbob @TylerLeonhardt