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

Ctrl + L should be bound to clear repl by default #66302

Closed
octref opened this issue Jan 9, 2019 · 14 comments
Closed

Ctrl + L should be bound to clear repl by default #66302

octref opened this issue Jan 9, 2019 · 14 comments
Assignees
Labels
feature-request Request for new features or functionality release-notes Release notes issues verification-needed Verification of issue is requested verified Verification succeeded

Comments

@octref
Copy link
Contributor

octref commented Jan 9, 2019

Ctrl + L should be bound to workbench.debug.panel.action.clearReplAction by default. This is the standard behavior on terminal.

@isidorn
Copy link
Contributor

isidorn commented Jan 10, 2019

Ctrl + L is a key chord combination and a user can enable it for any custom keybinding while also in debug console.
Since this is the first request for this I would ask you to customise this keybinding that Ctrl + L clears the console and if more users ask for this I would consider changing the default.
I can be convinced, but am not convinced yet.

fyi @weinand @roblourens

@isidorn isidorn added the feature-request Request for new features or functionality label Jan 10, 2019
@isidorn isidorn added this to the Backlog milestone Jan 10, 2019
@octref
Copy link
Contributor Author

octref commented Jan 10, 2019

@Tyriar bind Ctrl+L to clear terminal by default. Can we add this for inDebugRepl && textInputFocus or panelFocus && activePanel == 'workbench.panel.repl'?

@Tyriar
Copy link
Member

Tyriar commented Jan 10, 2019

ctrl+L is a common keybinding to clear shells, cmd+k is the only default keybinding to clear the terminal, ctrl+k are not defaults on LInux/Windows as it would break chords.

https://github.com/Microsoft/vscode/blob/14aa41aca780a28fdd992b9b66fce6e9de2207ab/src/vs/workbench/parts/terminal/electron-browser/terminal.contribution.ts#L371

I think it makes more sense to have ctrl+L as an option via custom keybindings since no one has requested it.

@octref
Copy link
Contributor Author

octref commented Jan 10, 2019

since no one has requested it.

Or, it might be that little people can discovered it.

Ctrl + L is a key chord combination and a user can enable it for any custom keybinding while also in debug console.

So this would only break people who run Ctrl+L in Debug Console and intending to invoke a custom chord he has defined.

I tried defining the following chord:

  {
    "key": "ctrl+l ctrl+n",
    "command": "workbench.action.focusNextGroup"
  }

And I already cannot invoke it in terminal. So I think it's fine to make debug console behave the same. People who really want to invoke Ctrl+L chords in terminal/debug-console can remove the builtin shortcut.

@Tyriar
Copy link
Member

Tyriar commented Jan 10, 2019

Why are we talking about chords? ctrl+l isn't used for built-in chords is it?

@octref
Copy link
Contributor Author

octref commented Jan 11, 2019

See @isidorn's comment.

@Tyriar
Copy link
Member

Tyriar commented Jan 11, 2019

Ctrl + L is a key chord combination and a user can enable it for any custom keybinding while also in debug console.

Can't any ctrl+key be a chord combination? 😕

@isidorn
Copy link
Contributor

isidorn commented Jan 11, 2019

afaik no, only ctrl + l and ctrl + k
@alexandrudima what keybdingins are reserved for chords?

@alexdima
Copy link
Member

@isidorn We don't reserve/hard code keybindings for chords, i.e. any valid keybinding can be the first part of a chord.

@isidorn
Copy link
Contributor

isidorn commented Jan 14, 2019

@alexandrudima I was unaware of that, thanks.
@octref ok, let go with ctrl + L to clear debug console.

fyi @weinand

@isidorn
Copy link
Contributor

isidorn commented Jan 14, 2019

@octref sorry I changed my mind, since I only now figured out that we do not even use ctrl+l to clear the terminal (only now read @Tyriar comment).
Since we are not even consistent in the terminal, we will not do anything for the debug console.

If we were to do something for the debug console should we use the same as the terminal or ctrl+L as you suggest.
For now I decide to keep it wihtout a keybidning.

@octref
Copy link
Contributor Author

octref commented Jan 14, 2019

@isidorn We do clear terminal on Ctrl+L although it's not specified as a shortut. So does Ctrl+A/E for going to beginning/end of current command.

Chrome DevTools also have it.

@Tyriar
Copy link
Member

Tyriar commented Jan 14, 2019

Maybe you should bring it up in the UX call to see what people think? I'd be interested in seeing which editors/IDEs use this for repls.

@isidorn
Copy link
Contributor

isidorn commented Jan 15, 2019

@octref thanks for providing more details. I am finally convinced, since we are clearing with Ctrl + L the terminal it makes sense to do this in the debug console.

To verify:
Have some output in the debug console and make sure that pressing ctrl + L clears it
Also make sure that pressing ctrl + L when the focus is not in the debug console has no effect.

@isidorn isidorn added the verification-needed Verification of issue is requested label Jan 15, 2019
@RMacfarlane RMacfarlane added the verified Verification succeeded label Jan 30, 2019
@isidorn isidorn added the release-notes Release notes issues label Feb 1, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality release-notes Release notes issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@isidorn @Tyriar @RMacfarlane @octref @alexdima and others