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

vscode: Support location in TerminalOptions #54

Closed
wants to merge 4 commits into from
Closed

Conversation

xai
Copy link
Member

@xai xai commented Nov 2, 2022

What it does

Support location in TerminalOptions, whereas location can be

  • a TerminalLocation enum with the values Editor and Panel
  • a TerminalEditorLocationOptions object which contains a ViewColumn and an optional preserveFocus boolean
    ViewColumn can be Active, Besides, or an explicit column from One to Nine
  • a TerminalSplitLocationOptions object which contains a parentTerminal that is supposed to be split.

Contributed on behalf of STMicroelectronics

Fixes eclipse-theia#11506

How to test

  • Download test extension and copy it into the plugins folder.
  • Start Theia and verify that the commands provided by the test extension trigger the correct1 behavior:
    • Open terminal in editor opens a terminal in the main area
    • Open terminal in panel opens a terminal in the bottom area
    • Open terminal with TerminalSplitLocationOptions splits the provided parent terminal, the test extension uses the most recent one as parent argument
      Apparently, this one is currently broken in VS Code and just opens a terminal/tab in the panel area without splitting anything
    • Open terminal with TerminalEditorLocationOptions provides quick picks for ViewColumn and PreserveFocus
      • ViewColumn 'Active' should open terminal in the current column
        Apparently this is currently broken in VS Code, it seems to always open a tab in the first column. No idea what is actually supposed to happen - maybe tab in active column if there already is a terminal?
      • ViewColumn 'Besides' should open a terminal to the right of the current terminal
      • ViewColumn should open a terminal in the specified column if possible
      • PerserveFocus specifies whether the focus should remain untouched (default is false)

Review checklist

Reminder for reviewers

Footnotes

  1. The VSCode API specification and the current behavior of VSCode seems to be misaligned and at least partly broken in VSCode.

@xai xai force-pushed the issues/11506 branch 5 times, most recently from 727976d to 8f33bf7 Compare December 14, 2022 16:50
* Add support for TerminalOptions.location
* Implement TerminalLocation
* Implement TerminalSplitLocationOptions
* Implement TerminalEditorLocationOptions
* Keep (bottom area aka TerminalLocation.Panel) as default target

Fixes eclipse-theia#11506

Contributed on behalf of STMicroelectronics

Signed-off-by: Olaf Lessenich <[email protected]>
@xai xai marked this pull request as ready for review December 14, 2022 17:10
@xai xai changed the title WIP: vscode: Support location in TerminalOptions vscode: Support location in TerminalOptions Dec 14, 2022
@xai xai marked this pull request as draft December 14, 2022 18:00
@xai xai marked this pull request as ready for review December 16, 2022 14:27
Copy link

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

I had a look at the code and have some minor remarks/questions. I will test this beginning of next week.

Co-authored-by: Lucas Koehler <[email protected]>
Signed-off-by: Olaf Lessenich <[email protected]>
Copy link

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

I tested the changes with the provided extension and it worked well for me.

I added one more minor inline suggestion.

packages/plugin-ext/src/main/browser/terminal-main.ts Outdated Show resolved Hide resolved
@xai xai closed this Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vscode] Support specifying the location in terminal options
2 participants