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

Add top-level nested commands #7174

Closed
zadjii-msft opened this issue Aug 4, 2020 · 4 comments · Fixed by #7348
Closed

Add top-level nested commands #7174

zadjii-msft opened this issue Aug 4, 2020 · 4 comments · Fixed by #7348
Assignees
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Question For questions or discussion Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@zadjii-msft
Copy link
Member

I'm filing this one to get team feedback for what commands we should include in the defaults once #6856 lands. I'm gonna throw the usual suspects on the Assigned To line, and remove yourself when you've shared thoughts


Shouldn't we replace the "newTab" and "splitPane" entries here with nested/iterable commands?

I was thinking, something neat would be to make the tree look like this:

<Command Palette>
└─ Open Profile...
   ├─ Profile 1...
   |  ├─ New Tab
   |  ├─ Split Automatically
   |  ├─ Split Vertically
   |  └─ Split Horizontally
   ├─ Profile 2...
   |  ├─ New Tab
   |  ├─ Split Automatically
   |  ├─ Split Vertically
   |  └─ Split Horizontally
   └─ Profile 3...
      ├─ New Tab
      ├─ Split Automatically
      ├─ Split Vertically
      └─ Split Horizontally

_Originally posted by @carlos-zamora in #6856


Yea, that's what I was thinking too, but I think I wanted to open a broader discussion with the team for what these commands should be. I was thinking that there's be two top-level ones (Split pane... and New Tab...), but I want a team consensus on those.


In my head, this is how I pictured it:

  {
    "name": "New tab...",
    "commands": [
      {
        "iterateOn": "profiles",
        "icon": "${profile.icon}",
        "name": "${profile.name}",
        "command": { "action": "newTab", "profile": "${profile.name}" }
      }
    ]
  },
  {
    "name": "Split pane...",
    "commands": [
      {
        "iterateOn": "profiles",
        "icon": "${profile.icon}",
        "name": "${profile.name}...",
        "commands": [
          {
            "name": "Split automatically",
            "command": { "action": "splitPane", "profile": "${profile.name}", "split": "automatic" }
          },
          {
            "name": "Split vertically",
            "command": { "action": "splitPane", "profile": "${profile.name}", "split": "vertical" }
          },
          {
            "name": "Split horizontally",
            "command": { "action": "splitPane", "profile": "${profile.name}", "split": "horizontal" }
          }
        ]
      }
    ]
  }

Which then would evaluate to

<Command Palette>
├─ New tab...
|  ├─ Profile 1
|  ├─ Profile 2
|  └─ Profile 3
└─ Split pane...
   ├─ Profile 1...
   |  ├─ Split Automatically
   |  ├─ Split Vertically
   |  └─ Split Horizontally
   ├─ Profile 2...
   |  ├─ Split Automatically
   |  ├─ Split Vertically
   |  └─ Split Horizontally
   └─ Profile 3...
      ├─ Split Automatically
      ├─ Split Vertically
      └─ Split Horizontally
@zadjii-msft zadjii-msft added Issue-Question For questions or discussion Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Aug 4, 2020
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Aug 4, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 4, 2020
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 4, 2020
@cinnamon-msft
Copy link
Contributor

cinnamon-msft commented Aug 18, 2020

I prefer the layout of option 2. However, I could see a use case for people searching for a specific profile and the default behavior would be to open a new tab.

@leonMSFT
Copy link
Contributor

I like 2 better as well, its layout seems more logical to me. IMO I feel like if I wanted to open a New Tab or Split Pane (with a specific profile or not), I'd look through the cmdpal for those commands verbatim instead of looking for Open Profile.

In addition to the nested New Tab and Split Pane, I'd still like to leave in a non-nested version of New Tab and Split Pane that do their default behaviors for quick one-shot enter keypresses instead of double/triple keypress to navigate through the nested versions.

@leonMSFT leonMSFT removed their assignment Aug 18, 2020
@zadjii-msft
Copy link
Member Author

Alright well that's enough consensus for me to go on. I'll ship a PR for option 2, and we'll finish discussion in the PR. Thanks guys!

@ghost ghost added the In-PR This issue has a related PR label Aug 19, 2020
@ghost ghost closed this as completed in #7348 Aug 21, 2020
ghost pushed a commit that referenced this issue Aug 21, 2020
## Summary of the Pull Request
![cmdpal-default-nested-commands](https://user-images.githubusercontent.com/18356694/90684483-e6b13400-e22d-11ea-8ca6-fe90ca8d9e82.gif)

Adds a pair of top-level commands that both have nested, iterable sub-commands. The "New Tab..." command has one child for each profile, and will open a new tab for that profile. The "Split Pane..." command similarly has a nested command for each profile, and also has a nested command for split auto/horizontal/vertical.

## References

* megathread: #5400 
* Would look better with icons from  #6644

## PR Checklist
* [x] Closes #7174 
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 21, 2020
@ghost
Copy link

ghost commented Aug 26, 2020

🎉This issue was addressed in #7348, which has now been successfully released as Windows Terminal Preview v1.3.2382.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Question For questions or discussion Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants