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.

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

Teach command palette to fill in selected commandline upon right arrow #11069

Merged
2 commits merged into from
Aug 30, 2021

Conversation

Don-Vito
Copy link
Contributor

Closes #11049

@ghost ghost added Area-CmdPal Command Palette issues and features Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Aug 27, 2021
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

Okay checked out the branch - I love it. This is exactly what I wanted ☺️ Granted, I should make sure the rest of the team is on board with this UX pattern, but I love it

@DHowett
Copy link
Member

DHowett commented Aug 30, 2021

Can one-a-ya share a GIF? 😄

@zadjii-msft
Copy link
Member

gh-11069

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Aug 30, 2021
@DHowett
Copy link
Member

DHowett commented Aug 30, 2021

/me smashes that merge button

_searchBox().Select(_searchBox().Text().size(), 0);
_searchBox().Focus(FocusState::Programmatic);
_filteredActionsView().SelectedIndex(-1);
e.Handled(true);
Copy link
Member

Choose a reason for hiding this comment

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

Same question as @Rosefield, actually -- this doesn't break left/right in the general case, right? For editing the command once you pop it in the box?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the change as is is fine since it only handles the right array key if you have something selected, and it deselects the option after you hit right arrow the first time.

@DHowett
Copy link
Member

DHowett commented Aug 30, 2021

@msftbot merge this in 1 minute

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 30, 2021
@ghost
Copy link

ghost commented Aug 30, 2021

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Mon, 30 Aug 2021 18:35:04 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@ghost ghost merged commit 871b8de into microsoft:main Aug 30, 2021
@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Aug 30, 2021
ghost pushed a commit that referenced this pull request Aug 31, 2021
The first time you open commandline mode, `recentCommands` doesn't exist yet. However, we immediately try to read the `Size()` in a couple places. This'll A/V and we'll crash 😨 

The fix is easy - don't try and read the size of the non-existent `recentCommands`

Found this while playing with #11069
Regressed in #11030 
Didn't bother filing an issue for it when I have the fix in hand
@ghost
Copy link

ghost commented Oct 20, 2021

🎉Windows Terminal Preview v1.12.2922.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CmdPal Command Palette issues and features AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pressing the right arrow in the commandline mode of the cmdpal should fill in the selected command
4 participants