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

terminal: add context menu #12326

Merged

Conversation

planger
Copy link
Contributor

@planger planger commented Mar 20, 2023

What it does

Adds a context menu to the terminal widget that covers roughly the same menu items as VS Code.

  • New Terminal
  • Split Terminal
  • Copy
  • Paste
  • Select All
  • Clear
  • Kill Terminal

Therefore, this change also creates the two new commands Select All and Kill Terminal.

Fixes #7632
Contributed on behalf of STMicroelectronics.
Change-Id: Icb23f6fbdc44cdc5e018277e44ed5fe46c8ac26a

How to test

Open a terminal and try out all context menu items (list see above).
It may be worth making sure that the commands also work when triggered via the command palette.

Review checklist

Reminder for reviewers

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that the functionality works as expected 👍
I only have some very minor comments regarding the code.

@vince-fugnitto vince-fugnitto added the terminal issues related to the terminal label Mar 21, 2023
@planger planger force-pushed the planger/issues/7632 branch from 306e8fa to 713824b Compare March 21, 2023 16:15
@planger planger requested a review from vince-fugnitto March 21, 2023 16:18
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I can confirm that the context-menu is correctly rendered on both the browser and electron, and the commands work as expected.

@planger planger force-pushed the planger/issues/7632 branch from 713824b to 9165ff3 Compare March 23, 2023 08:00
Contributed on behalf of STMicroelectronics.
Fixes eclipse-theia#7632

Change-Id: Icb23f6fbdc44cdc5e018277e44ed5fe46c8ac26a
@planger planger force-pushed the planger/issues/7632 branch from 9165ff3 to c246077 Compare March 23, 2023 20:14
@planger
Copy link
Contributor Author

planger commented Mar 23, 2023

c246077 rebases on top of current master to resolve conflicts

@JonasHelming JonasHelming merged commit 441c5a1 into eclipse-theia:master Mar 24, 2023
@vince-fugnitto vince-fugnitto added this to the 1.36.0 milestone Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
terminal issues related to the terminal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terminal: add context-menu
3 participants