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-c (terminate) not working in Terminals #7836

Closed
tsmaeder opened this issue May 19, 2020 · 4 comments · Fixed by #7839
Closed

Ctrl-c (terminate) not working in Terminals #7836

tsmaeder opened this issue May 19, 2020 · 4 comments · Fixed by #7839
Assignees
Labels
keybindings issues related to keybindings

Comments

@tsmaeder
Copy link
Contributor

Bug Description:

When I run a task and type <ctrl>-c in the terminal, the task is not interrupted

Steps to Reproduce:

  1. Start any long running task
  2. Click in the terminal window executing said task to give it focus
  3. Type <ctrl>-c?
  4. Observe: the task continues to execute

My expectation would be that the task be interrupted, like in a regular shell.

Additional Information

  • Operating System: Windows 10, Chromium, but also does not work on Linux
  • Theia Version: Master
@tsmaeder
Copy link
Contributor Author

A bisection of commits was able to trace this back to this commit: 62c9a83

@akosyakov akosyakov self-assigned this May 19, 2020
@akosyakov akosyakov added the keybindings issues related to keybindings label May 19, 2020
@akosyakov
Copy link
Member

akosyakov commented May 19, 2020

So 62c9a83 makes sure that we don't mess with an order of registered keybindings, i.e. the logic should be a keybinding in the higher scope should have a priority and within a scope keybinding which registered later should have a priority. The code before randomly puts some keybindings with context property as the higher priority disregarding scopes and order of registration.

I think the proper solution will be to delete DEFAULT_OVERRIDING scope and evaluate all keybindings in the reverse order not only for DEFAULT_OVERRIDING scope as now. I will send a PR soon for testing. I hope we don't break something else with it.

@tsmaeder
Copy link
Contributor Author

@akosyakov it's all greek to me 🤷, that's why I did not propose a fix.

akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this issue May 19, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@marcdumais-work
Copy link
Contributor

it's all greek to me

Pretty much :) But I guesstimate that, given the context, another shortcut is currently "winning" the battle and overriding the terminate one we all know and love. Perhaps copy?

akosyakov added a commit that referenced this issue May 20, 2020
…luation

Also match only first keybinding on keydown, don't traverse over all keybindings.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keybindings issues related to keybindings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants