-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Allow commands to be previewed in the command palette #6689
Comments
I wonder if it's as "easy" as having the "active TerminalSettings context" and manipulating that, and then either reverting to the existing one or committing the new one as the active one. |
So like, the command always manipulates (permanently!) the state. We just make a copy, and discard it to undo. |
That's so so so much easier |
## Summary of the Pull Request Adds the `setColorScheme` action, to change the color scheme of the active control to one given by the `name` parameter. `name` is required. If `name` is not the name of a color scheme, the action does nothing. ## References * Being done as a stepping stone to #6689 ## PR Checklist * [x] Closes #5401 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments Technically, the action is being done by changing the settings of the current `TerminalSettings` of the `TermControl`. Frankly, it should be operating on a copy of the `TermControl`'s `IControlSettings`, then updating the control's settings, or the Control should just listen for changes to it's setting's properties, and update in real time (without a manual call to `UpdateSettings`. However, both those paths are somewhere unknowable beyond #6904, so we'll just do this for now. ## Validation Steps Performed * tested manually with a scheme that exists * tested manually with a scheme that doesn't exist
## 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.
## Summary of the Pull Request ![preview-ats-000](https://user-images.githubusercontent.com/18356694/94801728-18302a00-03ac-11eb-851d-760b92ebb46f.gif) This PR enables the ATS to display the active tab as the user navigates the tab switcher. We do this by dispatching the tab switch actions as the user navigates the menu, and manually _not_ focusing the new tab when the tab switcher is open. ## References * #6732 - original tab switcher PR * #6689 - That's a more involved, generic version of this, but this PR will be enough to stop most of the complaints hopefully ## PR Checklist * [x] Closes #7409 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Opened tabs, tabbed through the menu, verified that it did what I'd expect
## Summary of the Pull Request ![preview-ats-000](https://user-images.githubusercontent.com/18356694/94801728-18302a00-03ac-11eb-851d-760b92ebb46f.gif) This PR enables the ATS to display the active tab as the user navigates the tab switcher. We do this by dispatching the tab switch actions as the user navigates the menu, and manually _not_ focusing the new tab when the tab switcher is open. ## References * #6732 - original tab switcher PR * #6689 - That's a more involved, generic version of this, but this PR will be enough to stop most of the complaints hopefully ## PR Checklist * [x] Closes #7409 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Opened tabs, tabbed through the menu, verified that it did what I'd expect (cherry picked from commit 22887d7)
## Summary of the Pull Request Allow schemes to be previewed as the user hovers over them in the Command Palette. ![preview-set-color-scheme](https://user-images.githubusercontent.com/18356694/114557761-9a3cbd80-9c2f-11eb-987f-eb0c89ee1fa6.gif) ## References * Branched off of #8392, which is why the commit history is so polluted. 330a8e8 : 544b2fd has the interesting commits * #5400: cmdpal megathread ### Potential follow-ups * changing the font size * changing the font face * changing the opacity of acrylic ## PR Checklist * [x] Closes #6689, a last straggling FHL PR * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated - I don't think so ## Detailed Description of the Pull Request / Additional comments This works by inserting a "preview" `TerminalSettings` into the settings hierarchy, before the `TermControl`'s runtime settings, and after the ones from the actual `CascadiaSettings`. This allows us to modify that preview settings object, then discard it when we're done with the preview. This could also be used for other settings in the future - I built it to be extensible to other `ShortcutAction`s, though I haven't implemented those yet. ## Validation Steps Performed * Select a colorscheme - it becomes the active one * `colortool -x <scheme>` after selecting a scheme - colortool overrides the selected scheme * Select a colorscheme after a `colortool -x <scheme>` after selecting a scheme - the scheme in the palette becomes the active one * Pressing <kbd>esc</kbd> at any point to dismiss the command palette - scheme returns to the previous one * reloading the settings - returns to the scheme in the settings
🎉This issue was addressed in #9794, which has now been successfully released as Handy links: |
Use cases:
These would need to be commands that could be undo-able. Commands where we could build a "reset" command given the current state of the Terminal, and hitting esc to dismiss the palette would revert to the original state.
I'd imagine that not all commands could be previewed. Can't preview "open a new tab".
If you went from one previewable command to another of the same action type, then we don't need to reset in between. But going from a "see theme to foo"->"set opacity to 5%" commands, we'd need to undo the first, then cache the state for the second command.
Could probably have some
ActionArgs
implementIPreviewable
, with aActionAndArgs GenerateUndoCommand(AppLogic);
method. Anything that's"preview":true
would need to implement that interface.</showerthought>
Considered use cases:
The text was updated successfully, but these errors were encountered: