-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 a 'splitPane' ShortcutAction #3722
Conversation
…ther vertically or horizontally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor stuff that should be fixed but won't take too long.
const std::string bindings0String{ R"([ | ||
{ "command": "splitVertical", "keys": ["ctrl+a"] }, | ||
{ "command": "splitHorizontal", "keys": ["ctrl+b"] }, | ||
{ "command": { "action": "splitPane", "split": null }, "keys": ["ctrl+c"] }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, what does this do when you have split = null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just for warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split=null will default to SplitState::None
which actually does nothing. There's another issues laying around for validating that keybindings have args with actual reasonable values and warning if you don't
…ane-action # Conflicts: # src/cascadia/TerminalApp/AppKeyBindings.cpp # src/cascadia/TerminalApp/AppKeyBindings.h # src/cascadia/TerminalApp/AppKeyBindings.idl # src/cascadia/TerminalApp/TerminalPage.cpp
🎉 Handy links: |
Summary of the Pull Request
We already have "splitHorizontal" and "splitVertical", but those will both be deprecated in favor of "splitPane" with arguments.
Currently, there's one argument: "style", which is one of "vertical" or "horizontal."
References
This is being done in pursuit of supporting #607 and #998. I don't really want to lob #998 in with this one, since both that and this are hefty enough PRs even as they are. (I have a branch for #998, but it needs this first)
This will probably conflict with #3658
PR Checklist
Validation Steps Performed
Added new keybindings with the args - works
Tried the old keybindings without the args - still works