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

Cmd+K for clearing the terminal doesn't work #43623

Closed
fabiospampinato opened this issue Feb 13, 2018 · 16 comments
Closed

Cmd+K for clearing the terminal doesn't work #43623

fabiospampinato opened this issue Feb 13, 2018 · 16 comments
Assignees
Labels
keybindings VS Code keybinding issues terminal Integrated terminal issues under-discussion Issue is under discussion for relevance, priority, approach

Comments

@fabiospampinato
Copy link
Contributor

fabiospampinato commented Feb 13, 2018

VS Code version: Code 1.20.0 (c63189d, 2018-02-07T17:02:34.244Z)
OS version: Darwin x64 17.5.0

There's this object in my default keybindings (macOS):

{ "key": "cmd+k",                 "command": "workbench.action.terminal.clear",
                                     "when": "terminalFocus" },

It's supposed to clear the terminal on cmd+k, basically the same behaviour of the default Terminal.app.

This doesn't work though, since cmd+k is not considered a complete shortcut on its own and VSC will wait for another key/shortcut to complement it.

I think a timeout should be implemented in the shortcuts handler, if no other key is pressed within N milliseconds execute what has been typed so far as a shortcut, in this case clear the terminal instead of waiting forever.

@vscodebot vscodebot bot added the terminal Integrated terminal issues label Feb 13, 2018
@shobhitchittora
Copy link
Contributor

@fabiospampinato this works for me in both the latest VSCode and Insiders. Can you specify which one are you using?

This doesn't work though, since cmd+k is not considered a complete shortcut on its own and VSC will wait for another key/shortcut to complement it.

How did you reach to this conclusion?

@fabiospampinato
Copy link
Contributor Author

fabiospampinato commented Feb 14, 2018

@shobhitchittora I've added some info about the VSC version I'm using to my original post.

I'm pretty surprised this works for you. I think it never worked for me, it was for sure broken in the past ~5 releases for me.

How did you reach to this conclusion?

There are plenty of keybindings listening for shortcuts like cmd+k cmd+*, besides I can press cmd+k, wait an arbitrary amount of time, then press a and the a character doesn't get printed out anywhere, I'm assuming because the shortcuts handler is catching that keypress.

@Tyriar
Copy link
Member

Tyriar commented Feb 14, 2018

@fabiospampinato this is a common issue, it happens when either you define a ctrl+k chord keybinding or an extension does it. The only workaround currently is to redefine the cmd+k keybinding at a higher level (user keybindings.json). Adding this again to the bottom of your keybindings.json should fix the problem:

{ "key": "cmd+k",                 "command": "workbench.action.terminal.clear",
                                     "when": "terminalFocus" },

This is the best we can do at the moment due to how the keybindings system works. It's a little hacky that it works at all, a special case was added just to get terminal clear working.

@Tyriar Tyriar closed this as completed Feb 14, 2018
@fabiospampinato
Copy link
Contributor Author

@Tyriar Thanks for the info.

I don't think this can be considered fixed though, I'm still having a few problems:

Multiple commands triggered

My keybindings.json:

[
  { "key": "cmd+k cmd+b", "command": "workbench.action.toggleSidebarVisibility" },
  { "key": "cmd+k", "command": "workbench.action.terminal.clear", "when": "terminalFocus" }
]
  • If I focus on a terminal and do cmd+k cmd+b very fast I expect that only workbench.action.toggleSidebarVisibility will be executed, but workbench.action.terminal.clear is executed too.

Order taken into account

My keybindings.json (same as before but in reverse order):

[
  { "key": "cmd+k", "command": "workbench.action.terminal.clear", "when": "terminalFocus" },
  { "key": "cmd+k cmd+b", "command": "workbench.action.toggleSidebarVisibility" }
]
  • If I focus on a terminal and do cmd+k the terminal doesn't get cleared.

  • If I focus on a terminal and do cmd+k cmd+b the sidebar gets toggled, but the terminal doesn't get cleared. In the default keybindings.json there are other cmd+k * shortcuts defined after the cmd+k shortcut so I don't understand why this is not behaving consistently.

  • I can focus on a terminal, docmd+k, wait 10 seconds, then press a and nothing gets printed. I presume because it gets recognized as a shortcut, but what kind of shortcut is done in 10 seconds or more... This problem doesn't happen if my keybindings.json is []

@Tyriar
Copy link
Member

Tyriar commented Feb 28, 2018

So most of you said is how I expect it to work currently, except for this:

If I focus on a terminal and do cmd+k cmd+b very fast I expect that only workbench.action.toggleSidebarVisibility will be executed, but workbench.action.terminal.clear is executed too.

I cannot repro this but it's a bug if it happens. Any more steps on how to repro would be good.


But yeah it's a bit confusing, currently the behavior is chord keybindings always win unless there is a keybinding with a higher priority than them. We also want to make it easy for users to unbind cmd+k as some users will want to use chord keybindings even when the terminal is focused.

@alexandrudima it's been a while since you looked at this, has there been any movement around the keybindings system that would make it easier for cmd+k to get a higher priority due to the when clause? I get around an issue every 1-2 weeks on this because extensions or users add extra chord keybindings.

@Tyriar Tyriar reopened this Feb 28, 2018
@Tyriar Tyriar added keybindings VS Code keybinding issues under-discussion Issue is under discussion for relevance, priority, approach labels Feb 28, 2018
@alexdima
Copy link
Member

alexdima commented Mar 1, 2018

@Tyriar

If someone adds a user keybinding for cmd+k cmd+b, with a when clause that is true when terminalFocus, then that keybinding will win over the default clear terminal cmd+k and everytime cmd+k is pressed, the system will look for a chord.

The keybinding system is a rule-based system, where rules are evaluated in the order that users write them. See https://code.visualstudio.com/docs/getstarted/keybindings#_keyboard-rules . Even if some of the when clauses might looks like locations (e.g. terminalFocus holds only when in the terminal), very many other when clauses are not locations. So, the keybinding system is not a CSS-like, element-selector-like based system, that would prioritize rules based on how one selects an element, because the when clause is not a selector.

There are no plans whatsoever to change this.

@fabiospampinato
Copy link
Contributor Author

fabiospampinato commented Mar 1, 2018

@Tyriar

Any more steps on how to repro would be good.

This is exactly what I'm doing:

  • Replace the content of keybindings.json with:
[
  { "key": "cmd+k cmd+b", "command": "workbench.action.toggleSidebarVisibility" },
  { "key": "cmd+k", "command": "workbench.action.terminal.clear", "when": "terminalFocus" }
]
  • Execute code --disable-extensions ~/Desktop and focus to this newly opened window
  • ctrl+` for opening a terminal (Notice how the terminal doesn't receive the focus, but if you kill it and do ctrl+` again it will receive the focus. This looks like another bug to me, do you want me to open a separate issue about it?)
  • Execute ls in the terminal
  • cmd+k cmd+b, the terminal gets cleared and the sidebar gets toggled.

@alexandrudima

I think at least it should be stated in the docs that if you add any keybinding listening for cmd+k * you'll have to also redefine { "key": "cmd+k", "command": "workbench.action.terminal.clear", "when": "terminalFocus" } or it won't work anymore.

The lack of a timeout for chord shortcuts is also confusing to me, I was expecting that after doing a cmd+k VSC would wait a fixed amount of time for another shortcut (in case I'm about to trigger cmd+k cmd+b or something like that) and if no other key gets pressed, cmd+k itself would be considered a shortcut. Instead VSC just waits forever.

I think @Tyriar was referring to the simpler case where a keybinding doesn't have a when clause defined and another does, in some sense making the latter more specific.

@alexdima
Copy link
Member

alexdima commented Mar 1, 2018

@fabiospampinato There is a default keybinding of cmd+b which toggles the sidebar.

So when you do cmd+k, cmd+b and you are focused in the terminal, you most likely end up:

  1. cmd+k -> clearing the terminal
  2. cmd+b -> toggle the sidebar

Also, most likely there is no message (cmd+k) was pressed. Waiting for second key of chord....


You have a point, that a chord should no longer be waited for after e.g. 1 minute or after focusing out of vscode. i.e. the state should be cleared. But that does not mean cmd+k should be re-dispatched and chords skipped after a set amount of time. i.e. the conflict between a chord cmd+k * and cmd+k keybinding should IMHO not be resolved based on passage of time. I have extracted this in #44847

@Tyriar
Copy link
Member

Tyriar commented Mar 1, 2018

@alexandrudima

If someone adds a user keybinding for cmd+k cmd+b, with a when clause that is true when terminalFocus, then that keybinding will win over the default clear terminal cmd+k and everytime cmd+k is pressed, the system will look for a chord.

This would be fine if it just worked like this, but even with no when clause is used it's overridden. So an extension could contribute a chord keybinding as that's a common thing in VS/VS Code such as:

{ "key": "cmd+k m", "command": "workbench.action.editor.changeLanguageMode" }

And now the keybinding that everyone knows for the terminal doesn't work. I think we discussed when you were over here that forcing then when to have higher priority would break many other keybindings so maybe a fix for this would be a priority similar to !important in CSS? I think the priorities are only internal at the moment and a default keybinding cannot be higher than a user keybinding.

So when you do cmd+k, cmd+b and you are focused in the terminal, you most likely end up:

  1. cmd+k -> clearing the terminal
  2. cmd+b -> toggle the sidebar

Ah this makes sense.

@alexdima
Copy link
Member

alexdima commented Mar 1, 2018

@Tyriar

This would be fine if it just worked like this, but even with no when clause is used it's overridden.

when: "" holds true always. when: "" is true when terminalFocus. These are not CSS selectors, this is binary logic. I don't know how I should explain this better. It's like driving with no restrictions on the German auto-bahn. The lack of a speed limit sign means you can drive with any speed. The lack of a when condition means that it is always going to be true.

@alexdima
Copy link
Member

alexdima commented Mar 1, 2018

@Tyriar

I thought of a better example.

(p1). I like to drink beer when it is hot outside.
(p2). I like to drink beer.

(p2) implies (p1) even if (p2) omits the when clause.

@Tyriar
Copy link
Member

Tyriar commented Mar 1, 2018

@alexandrudima but there's still the problem that cmd+k breaks all of a sudden when users install extensions or add seemingly innocuous keybindings when we want it to always be active unless explicitly disabled.

(p1). I really like to drink beer when it is hot outside.
(p2). I like to drink water. (contributed by an extension 😛)

@fabiospampinato
Copy link
Contributor Author

Leaving beer aside for a moment, look at what happens with this keybindings.json:

[
  { "key": "cmd+k cmd+b", "command": "workbench.action.toggleStatusbarVisibility" },
  { "key": "cmd+k", "command": "workbench.action.terminal.clear", "when": "terminalFocus" }
]

I do cmd+k cmd+b and the statusbar doesn't get toggled, instead my terminal gets cleared and the sidebar gets toggled. If I reverse the order of those entries in the array then I can't clear the terminal with cmd+k anymore.

Basically there's no way to do chord shortcuts while the terminal is in focus, if you also want cmd+k to clear the terminal.

I'm not saying this is a huge deal, but it definitely doesn't look right to me.

@Tyriar
Copy link
Member

Tyriar commented Mar 1, 2018

Basically there's no way to do chord shortcuts while the terminal is in focus, if you also want cmd+k to clear the terminal.

@fabiospampinato I don't see this changing, it would be weirder imo for it to clear the terminal and then toggle status bar visibility. if you use the chords a lot you can always unbind cmd+k and bind it to something else:

{ "key": "cmd+k", "command": "-workbench.action.terminal.clear", "when": "terminalFocus" }

@alexdima
Copy link
Member

alexdima commented Mar 2, 2018

@fabiospampinato

Basically there's no way to do chord shortcuts while the terminal is in focus, if you also want cmd+k to clear the terminal.

Yes, that is the default experience out of the box. Think of the keybinding service as a state machine. When you press a keybinding the keybinding service must make a decision from three options:

  1. do nothing (and leave chord mode if in chord mode)
  2. go into chord mode and wait for second part of a chord.
  3. execute a command (and leave chord mode if in chord mode)

You can configure it to do anyone of these 3 things, but it cannot do two of those at the same time. It cannot both clear the terminal and enter chord mode when you are focused in the terminal and press cmd+k.


Let's get back to the original problem. I write in keybindings.json:

{ "key": "cmd+k cmd+c", "command": "editor.action.addCommentLine" },

And now I cannot clear terminal with cmd+k.

My question is: so what? This is as designed. Our keybinding system allows to do it, and that is legal. If you don't know what editing keybindings.json does, then please do not edit it.

Look, I can break a on my keyboard. I just added this to keybindings.json:

{ "key": "a", "command":"type", "args": { "text":"b"} }

Again, so what? I have the power, it is my responsibility to use it wisely and read the documentation to understand the consequences of what I am doing.


@Tyriar if you believe cmd+k is often asked about, then please add an entry in the FAQ at https://code.visualstudio.com/docs/getstarted/keybindings#_common-questions so we can point people to it. You can also direct people to stack overflow for questions.

@Tyriar
Copy link
Member

Tyriar commented Mar 4, 2018

microsoft/vscode-docs#1471

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
keybindings VS Code keybinding issues terminal Integrated terminal issues under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

4 participants