Skip to content

Add functionality to allow a Show-Command like panel in VS Code #697

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 14 commits into from
Nov 28, 2018
Merged

Add functionality to allow a Show-Command like panel in VS Code #697

merged 14 commits into from
Nov 28, 2018

Conversation

corbob
Copy link
Contributor

@corbob corbob commented Jul 8, 2018

This is changes required to accommodate the functionality of VS Code Feature Request 1405

TylerLeonhardt
TylerLeonhardt reviewed Jul 8, 2018
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.

I'm sooooooo excited to get this in :)

@rjmholt
Copy link
Contributor

rjmholt commented Jul 31, 2018

@corbob Does this still need the WIP tag? Or ready to review?

@corbob corbob changed the title WIP: Add functionality to allow a Show-Command like panel in VS Code Add functionality to allow a Show-Command like panel in VS Code Aug 1, 2018
@corbob
Copy link
Contributor Author

corbob commented Aug 1, 2018

@rjmholt I believe we're ready to review. I was mostly holding out to finish documenting the request and message classes which I've added now.

works on my 💻 🚢 it!

rjmholt
rjmholt reviewed Aug 10, 2018
View reviewed changes
@rjmholt
Copy link
Contributor

rjmholt commented Aug 10, 2018

Sorry it took me so long to review @corbob. Everything looks really good, I just left a couple of comments

@corbob
Copy link
Contributor Author

corbob commented Aug 14, 2018

I believe I've addressed most of the comments with the latest commit, but there is still the question of a few that I replied to.

@corbob corbob mentioned this pull request Aug 28, 2018
Merged
3 tasks
@corbob corbob changed the title Add functionality to allow a Show-Command like panel in VS Code WIP: Add functionality to allow a Show-Command like panel in VS Code Aug 28, 2018
@corbob
Copy link
Contributor Author

corbob commented Aug 28, 2018

Changing PR back to WIP to address the refactoring @tylerl0706 suggested.

@TylerLeonhardt
Copy link
Member

@corbob is this ready for us to take a look? I have a good feeling :)

@corbob
Copy link
Contributor Author

corbob commented Sep 29, 2018

I think I've got most of the stuff. Only thing I'm not 100% on is some of the comment based documentation parts.

@corbob corbob changed the title WIP: Add functionality to allow a Show-Command like panel in VS Code Add functionality to allow a Show-Command like panel in VS Code Oct 8, 2018
@TylerLeonhardt
Copy link
Member

@corbob it looks like there are a couple comments still in this PR. Can you take a look and see if you've covered them and resolve them/comment on them?

corbob added 7 commits November 5, 2018 22:18
@corbob
Adding Get Command for Command Explorer
2ef6a12
@corbob
Return PowerShell Commands
9b5dc33 
Return all commands but aliases. We're returning the same as show-command
@corbob
Sort the commands in PowerShell
ac120d1 
Sorted in PowerShell seems to have a negligible performance cost. This benefits us greatly as it means we don't need to sort in TypeScript. We can still sort if we want, but it's the default sort.
@corbob
Using SMA
b84bda8 
Add using for System.Management.Automation
@corbob
Add Get Command request type and handler
8452e6e
@corbob
Add comments documenting the purpose of the message and requests
7ce82bd
@corbob
Remove extra logging. Be explicit with types.
1e995f6
corbob added 4 commits November 5, 2018 22:21
@corbob
Remove GetAllCommands.
0e15f40 
>> Refactor into a single command. If nothing is passed, it will return all objects.
@corbob
Qualify the things
8d91d17
@corbob
cleanup of some things
f363d87
@corbob
Update summary for classes.
0a795d5
TylerLeonhardt
TylerLeonhardt reviewed Nov 21, 2018
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.

ok this is super close. Address our couple comments and this is ready for sure

TylerLeonhardt
TylerLeonhardt reviewed Nov 22, 2018
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.

Just a few nits. Otherwise this is ready

@corbob
Clean up some commented code.
097c48b 
Fully Qualify the namespace of stuffs...
Something about Async...
TylerLeonhardt
TylerLeonhardt approved these changes Nov 23, 2018
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.

hmmm I can't find anything else to complain about! Oh no! That must mean LGTM 🎉

Really looking forward to getting this in 😄

rjmholt
rjmholt approved these changes Nov 24, 2018
View reviewed changes
Copy link
Contributor

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

LGTM!

@rjmholt rjmholt merged commit c68ca39 into PowerShell:master Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkeithhill rkeithhill rkeithhill left review comments

@rjmholt rjmholt rjmholt approved these changes

@SeeminglyScience SeeminglyScience SeeminglyScience left review comments

@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.
5 participants
@corbob @rjmholt @TylerLeonhardt @rkeithhill @SeeminglyScience