Skip to content
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

Add an Exclusion filter to the Command Explorer. #1686

Merged
merged 4 commits into from
Jan 28, 2019
Merged

Add an Exclusion filter to the Command Explorer. #1686

merged 4 commits into from
Jan 28, 2019

Conversation

corbob
Copy link
Contributor

@corbob corbob commented Jan 8, 2019 •
edited

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
Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

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

package.json Outdated Show resolved Hide resolved
src/features/GetCommands.ts Outdated Show resolved Hide resolved
@corbob 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
Copy link
Contributor Author

corbob commented Jan 17, 2019

@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
Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM 😊

@TylerLeonhardt
Copy link
Member

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

@corbob
Copy link
Contributor Author

corbob commented Jan 18, 2019 •
edited

@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
Copy link
Member

cool cool. Looks good then!

@corbob
Copy link
Contributor Author

corbob commented Jan 20, 2019

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerLeonhardt TylerLeonhardt TylerLeonhardt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@corbob @TylerLeonhardt