Add support for iterable, nested commands #6856

zadjii-msft · 2020-07-09T17:52:09Z

Summary of the Pull Request

This PR adds support for both nested and iterable commands in the Command palette.

Nested commands: These are commands that include additional sub-commands. When the user selects on of these, the palette will update to only show the nested commands.
Iterable commands: These are commands what allow the user to define only a single command, which is repeated once for every profile. (in the future, also repeated for color schemes, themes, etc.)

The above gif uses the following json:

        {
            "name": "Split Pane...",
            "commands": [
                {
                    "iterateOn": "profiles",
                    "name": "Split with ${profile.name}...",
                    "commands": [
                        { "command": { "action": "splitPane", "profile": "${profile.name}", "split": "automatic" } },
                        { "command": { "action": "splitPane", "profile": "${profile.name}", "split": "vertical" } },
                        { "command": { "action": "splitPane", "profile": "${profile.name}", "split": "horizontal" } }
                    ]
                }
            ]
        },

References

PR Checklist

Closes Add nested commands to the Command Palette #3994
I work here
Tests added/passed
Requires documentation to be updated - Sure does, but we'll finish polishing this first.

Detailed Description of the Pull Request / Additional comments

We've now gotta keep the original json for a command around, so that once we know what all the profiles will be, we can expand the commands that need it.

We've also got to parse commands recursively, because they might have any number of child commands.

These together made the command parsing a lot more complicated, but it feels good so far.

Validation Steps Performed

wrote a bunch of tests
Played with it a bunch

…ommandPalette-v1

…erplay with this

…ommandPalette-v1

…ommand-Palette-v2 # Conflicts: # src/cascadia/TerminalApp/TerminalPage.h

…an use it

Don't die when we encounter an unexpected key Reload successfully

…adow on the menuflyout???

…thing that uses MUX

…used later

…ommand-Palette-v2 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.cpp # src/cascadia/TerminalApp/GlobalAppSettings.h

# Conflicts: # src/cascadia/TerminalApp/ActionAndArgs.cpp # src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp

… enter the nesting

src/cascadia/TerminalApp/Command.cpp

DHowett

just a couple more qqs

… so here he goes

DHowett

Wellp, let's move fast and break things!

miniksa

Yep. I still don't like rewriting JSON, of course, but I don't have a better idea. This works and using it looks awesome. Let's go with it.

ghost · 2020-08-13T21:22:39Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

## Summary of the Pull Request ![cmdpal-set-color-scheme](https://user-images.githubusercontent.com/18356694/90517094-8eddd480-e12a-11ea-8be4-8b6782d8d88c.gif) Allows for creating commands that iterate over the user's color schemes. Also adds a top-level nested command to `defaults.json` that allows the user to select a color scheme (pictured above). I'm not sure there are really any other use cases that make sense, but it _really_ makes sense for this one. ## References * #5400 - cmdpal megathread * made possible by #6856, _and support from viewers like you._ * All this is being done in pursuit of #6689 ## PR Checklist * [x] Closes wait what? I could have swore there was an issue for this one... * [x] I work here * [x] Tests added/passed * [ ] Requires documentation to be updated - okay maybe now I'll write some docs ## Detailed Description of the Pull Request / Additional comments Most of the hard work for this was already done in #6856. This is just another thing to iterate over. ## Validation Steps Performed * Played with this default command. It works great. * Added tests.

## Summary of the Pull Request ![cmdpal-set-color-scheme](https://user-images.githubusercontent.com/18356694/90517094-8eddd480-e12a-11ea-8be4-8b6782d8d88c.gif) Allows for creating commands that iterate over the user's color schemes. Also adds a top-level nested command to `defaults.json` that allows the user to select a color scheme (pictured above). I'm not sure there are really any other use cases that make sense, but it _really_ makes sense for this one. ## References * microsoft#5400 - cmdpal megathread * made possible by microsoft#6856, _and support from viewers like you._ * All this is being done in pursuit of microsoft#6689 ## PR Checklist * [x] Closes wait what? I could have swore there was an issue for this one... * [x] I work here * [x] Tests added/passed * [ ] Requires documentation to be updated - okay maybe now I'll write some docs ## Detailed Description of the Pull Request / Additional comments Most of the hard work for this was already done in microsoft#6856. This is just another thing to iterate over. ## Validation Steps Performed * Played with this default command. It works great. * Added tests.

ghost · 2020-08-26T17:04:30Z

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

Handy links:

Dustin L. Howett * Clear the last error before calling Mb2Wc in ConvertToW (GH-7391) * Update clang-format to 10.0 (GH-7389) * Add til::static_map, a constexpr key-value store (GH-7323) James Holderness * Refactor VT control sequence identification (CC-7304) Mike Griese * Compensate for VS 16.7, part 2 (GH-7383) * Add support for iterable, nested commands (GH-6856) Michael Niksa * Helix Testing (GH-6992) * Compensate for new warnings and STL changes in VS 16.7 (GH-7319) nathpete-msft * Fix environment block creation (GH-7401) Chester Liu * Add initial support for VT DCS sequences (CC-6328) Related work items: #28791050

zadjii-msft added 30 commits April 30, 2020 15:02

Add a ton of text regarding Commandline Mode vs Action Mode

c56eb8f

move this spec out of drafts/

fe640ff

last little todos for review

614d1b2

Merge remote-tracking branch 'origin/master' into dev/migrie/s/5400-C…

ead76d7

…ommandPalette-v1

Move this section to 'future considerations'

2e56436

Merge remote-tracking branch 'origin/master' into dev/migrie/s/5400-C…

0886e8f

…ommandPalette-v1

Add some notes about the advanced tab switcher and how that might int…

11130a4

…erplay with this

Merge remote-tracking branch 'origin/master' into dev/migrie/s/5400-C…

f53553f

…ommandPalette-v1

good bot

0672812

Blind port these files from the old branch

43bd483

Hook up all the parsing once again

75af4ca

Get names from the resources if provided as an object, not a string

33a9e32

This should have been in the previous commit

3627d8a

Hook up the parsing of ActionAndArgss to the command palette

0416a94

Merge remote-tracking branch 'origin/master' into dev/migrie/f/2046-C…

6905065

…ommand-Palette-v2 # Conflicts: # src/cascadia/TerminalApp/TerminalPage.h

Make the action names map public, so the ToJson in AKBSerialization c…

02f47f4

…an use it

Move the action into it's own sub-object

62b9a0d

Add a ton of default commands

207666e

Make sure to scroll the selected item into view

b88be45

Add duplicate pane to the default commands

00763fd

Add some stability

bc546db

Don't die when we encounter an unexpected key Reload successfully

Do this in WinRTUtils instead of hackily doing it manually

e1be26b

Rename private methods, fix wraparound logic

a309191

Return focus to the active control when closed

9905945

Try to do this with a shadow, but it crashes inexplicably

cf6e1f2

Well, this is neat, and works, but requires 18362 and also casts a sh…

fa93fdc

…adow on the menuflyout???

Turns out, the shadow on the menuflyout isn't my fault, it's on every…

1fbe8e4

…thing that uses MUX

Extract ActionAndArgs::FromJson into it's own class, so it can be re-…

6e6979a

…used later

Merge remote-tracking branch 'origin/master' into dev/migrie/f/2046-C…

829beda

…ommand-Palette-v2 # Conflicts: # src/cascadia/TerminalApp/GlobalAppSettings.cpp # src/cascadia/TerminalApp/GlobalAppSettings.h

Merge commit '6e6979abe' into dev/migrie/f/2046-Command-Palette-v2

487f33e

# Conflicts: # src/cascadia/TerminalApp/ActionAndArgs.cpp # src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp

Add a helper for find/replace-ing a string

57abe29

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 12, 2020

zadjii-msft added 2 commits August 12, 2020 15:47

Clean up our string handling substantially

9981126

minor nits, more tracing

c55d53e

zadjii-msft mentioned this pull request Aug 12, 2020

A nested cmdpal menu should show the parent command above the list of commands #7265

Closed

zadjii-msft added 3 commits August 12, 2020 16:23

some cleaner lifetime management for these variables

3269309

use a IMap to store subcommands, and then generate the vector when we…

933dc7e

… enter the nesting

Fix the localtests for commands

5198142

zadjii-msft assigned DHowett Aug 13, 2020

DHowett reviewed Aug 13, 2020

View reviewed changes

src/cascadia/TerminalApp/Command.cpp Outdated Show resolved Hide resolved

DHowett reviewed Aug 13, 2020

View reviewed changes

src/cascadia/TerminalApp/Command.cpp Outdated Show resolved Hide resolved

DHowett reviewed Aug 13, 2020

View reviewed changes

Dustin wanted every command to not necessarily have a subcommand map,…

7b014d9

… so here he goes

DHowett approved these changes Aug 13, 2020

View reviewed changes

zadjii-msft unassigned DHowett Aug 13, 2020

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Aug 13, 2020

ghost requested review from miniksa and PankajBhojwani August 13, 2020 17:23

miniksa approved these changes Aug 13, 2020

View reviewed changes

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 13, 2020

ghost merged commit dcc2799 into master Aug 13, 2020

ghost deleted the dev/migrie/f/command-palette-iterateOn branch August 13, 2020 21:22

This was referenced Aug 17, 2020

Add nested commands to the Command Palette #3994

Closed

Add support for commands iterable on color schemes #7329

Merged

cinnamon-msft mentioned this pull request Aug 20, 2020

Document 1.3 release features MicrosoftDocs/terminal#117

Closed

14 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for iterable, nested commands #6856

Add support for iterable, nested commands #6856

zadjii-msft commented Jul 9, 2020

DHowett left a comment

DHowett left a comment

miniksa left a comment

ghost commented Aug 13, 2020

ghost commented Aug 26, 2020

Add support for iterable, nested commands #6856

Add support for iterable, nested commands #6856

Conversation

zadjii-msft commented Jul 9, 2020

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

DHowett left a comment

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

miniksa left a comment

Choose a reason for hiding this comment

ghost commented Aug 13, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Aug 26, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.