[vscode] Support TextEditor#show() and TextEditor#hide() #11146 #11168

jfaltermeier · 2022-05-17T13:46:05Z

What it does

Add support for TextEditor#show() and TextEditor#hide()

VSCode-behaviour:
When using TextEditor.show(ViewColumn) VSCode opens the editor again in preview-mode.
When using a numbered ViewColumn it brings the editor to the front, if existing, or opens it in the respective column. If the column is not existing it will be created.
TextEditor.hide() closes the editor.

Gif from VSCode with below extension:

fixes #11146

Contributed on behalf of STMicroelectronics

How to test

I've created a small vscode-extension that used the API here: https://github.com/jfaltermeier/vscode-playground/blob/f290f4b2f00cc3460e45b525eb822fe6824d986d/text-editor-show-hide/src/extension.ts
The vsix may be downloaded from here: https://github.com/jfaltermeier/vscode-playground/releases/tag/text-editor-show-hide-0.0.1

Start Theia with this extension and try the "Test show editor beside", "Test show editor in column one", "Test show editor in column two", "Test show editor in column three", and "Test hide editor" commands.
These work on the vscode.window.activeTextEditor.

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

jfaltermeier · 2022-05-17T13:46:37Z

packages/plugin-ext/src/main/browser/editors-and-documents-main.ts

@@ -168,6 +169,19 @@ export class EditorsAndDocumentsMain implements Disposable {
    saveAll(includeUntitled?: boolean): Promise<boolean> {
        return this.modelService.saveAll(includeUntitled);
    }
+
+    hideEditor(id: string): Promise<void> {
+        for (const editorWidget of this.editorService.all) {


Before trying to remove this loop, is there an existing way to get the EditorWidget for a MonacoEditor that I have missed?

Since the editor data reported to the plugin side has gone through the EditorSnapshot system, I don't really see a way recover the editor meant other than to repeat the process.

jfaltermeier · 2022-05-17T13:53:25Z

I've faced some issues that are either caused by me misusing Theia API or are existing Bugs. The code uses the OpenerService of Theia in the end and I believe that the options I am passing should be correct.
It would be nice to get some feedback on this.

The two added methods are actually deprecated on VSCode, but still required to reach compatibility. So we might be able to live with some of the issues.

Currently it seems that only one Preview editor can be opened for the same document in Theia. Is this correct?
When I use "Split right" on a preview editor, the new editor is not opened in preview mode.
We could decide to open in non-preview mode when using TextEditor#show() as a mitigation.
When using the numbered ViewColumn it seems that this leads to a split-right in Theia. Is this a known issue?
When using numbered ViewColumns, opening the document in an existing column does not seem to work. Is this a known issue?

vince-fugnitto · 2022-05-17T15:08:27Z

@jfaltermeier

Currently it seems that only one Preview editor can be opened for the same document in Theia. Is this correct? When I use "Split right" on a preview editor, the new editor is not opened in preview mode. We could decide to open in non-preview mode when using TextEditor#show() as a mitigation.

As far as I can tell this is also the same behavior in VS Code. Splitting a preview editor will open the new editor normally and not as a preview.

When using the numbered ViewColumn it seems that this leads to a split-right in Theia. Is this a known issue?

How would we test this?

When using numbered ViewColumns, opening the document in an existing column does not seem to work. Is this a known issue?

How would we test this?

packages/editor/src/browser/editor-manager.ts

colin-grant-work · 2022-05-17T21:14:24Z

packages/plugin-ext/src/common/plugin-api-rpc.ts

@@ -1157,11 +1157,11 @@ export interface DecorationOptions {
 }

 export interface TextEditorsMain {
-    // $tryShowTextDocument(resource: UriComponents, options: TextDocumentShowOptions): Promise<string>;
+    $tryShowTextDocument(uri: UriComponents, options?: TextDocumentShowOptions): Promise<void>;
    $registerTextEditorDecorationType(key: string, options: DecorationRenderOptions): void;
    $removeTextEditorDecorationType(key: string): void;
    // $tryShowEditor(id: string, position: EditorPosition): Promise<void>;


Should we implement / delete this variant?

I will remove it. Looking at the arguments the idea might have been to show/split an existing editor to a tab bar.
I think this can be achieved with tryShowTextDocument as well

colin-grant-work · 2022-05-17T21:21:12Z

packages/plugin-ext/src/main/browser/editors-and-documents-main.ts

@@ -168,6 +169,19 @@ export class EditorsAndDocumentsMain implements Disposable {
    saveAll(includeUntitled?: boolean): Promise<boolean> {
        return this.modelService.saveAll(includeUntitled);
    }
+
+    hideEditor(id: string): Promise<void> {
+        for (const editorWidget of this.editorService.all) {


Since the editor data reported to the plugin side has gone through the EditorSnapshot system, I don't really see a way recover the editor meant other than to repeat the process.

colin-grant-work · 2022-05-17T21:29:40Z

Currently it seems that only one Preview editor can be opened for the same document in Theia. Is this correct?
When I use "Split right" on a preview editor, the new editor is not opened in preview mode.

Only one preview can be open at all. If any other editor is opened as a preview, the old preview will be closed and the new preview will replace it. Details here. It appears that VSCode allows one editor preview per column, rather than one total.

When using the numbered ViewColumn it seems that this leads to a split-right in Theia. Is this a known issue?

That appears to be a consequence of the implementation of toEditorOpenerOptions here. The initial default is split-right, and if the column requested is greater than the number of columns currently available, that default remains in place. Is this different from VSCode? Using your test command to open in column three when there is only one column currently results in split-right behavior in VSCode, as well.

When using numbered ViewColumns, opening the document in an existing column does not seem to work. Is this a known issue?

Based on the code in the method referred to above, this looks like it should work: if the column exists, the active editor in that column should be used as the ref for opening the new widget. I would guess that this is a consequence of the multi-editor tracking system: at the moment, the only way to force a new editor to open with the same URI as an existing editor is to go through the openToSide system, since that forces the generation of a new counter (ID) for the new editor, rather than the reuse of an existing editor. The code you've used to force a new editor if the split options are detected could be adapted to check for a widget ref, check whether the tabbar for that ref has an editor with the URI, and if not generate a new ID for the new editor, otherwise reuse the editor in that column. Not a very clean solution, but the most direct, I think.

colin-grant-work · 2022-05-17T23:14:38Z

I've pushed a commit here that should get the behavior closer to VSCode. Feel free to use it if it suits your needs.

Editor previews are now limited to one per tab bar, not one per application.
EditorManager.open figures out where the new editor would go and only reuses an existing editor targeting the same file if the new widget would be opened on that widget's tab bar. Otherwise, a new editor is opened.

jfaltermeier · 2022-05-18T09:13:59Z

Thank you @colin-grant-work . With your commit (which also addresses your suggested change in editor-manager.ts) the three functional issues reported above seem to work as expected:

Can I cherry-pick your change to this branch?

colin-grant-work · 2022-05-18T13:58:36Z

Can I cherry-pick your change to this branch?

Yes, by all means, feel free.

vince-fugnitto

@jfaltermeier @JonasHelming I'm not sure about implementing the deprecated api, can't we just stub it? Extensions really shouldn't be using this API and it has been deprecated in VS Code from what it seems like the start (when it was public). I traced the deprecation back 7 years:

vince-fugnitto · 2022-05-19T15:17:11Z

packages/editor-preview/src/browser/editor-preview-manager.ts

+                this.all.forEach((editor: EditorPreviewWidget) => {
+                    if (editor.isPreview) {
+                        editor.convertToNonPreview();
+                    }


I believe we should check the preference name before we perform this operation since it might be expensive.
At the moment EditorPreviewPreferences only has the one preference but this might change in the future and better to be defensive.

Thanks, I've added the check

jfaltermeier · 2022-05-23T08:25:13Z

@vince-fugnitto I agree that we should not spent much effort into fully implementing deprecated API. Here the actual implementation of the deprecated api is not a big code change however (see the first commit): fda5937
hide loops over all widgets of the factory to find the editor to hide, which is a stub-implementaton. And show delegates to existing API.
All the other changes are related to differences in behaviour between Theia and VSCode, but those could also be triggered by using the proposed non-deprecated API window.showTextDocument
Should I open a further issue for the changelog explaining those differences? Then this PR could close two issues.

vince-fugnitto · 2022-05-24T18:56:15Z

here the actual implementation of the deprecated api is not a big code change however (see the first commit): fda5937

@jfaltermeier but rather than do fda5937 at all and include code that is only triggered by deprecated API we can simply stub it (minimal changes). Do we really want to maintain an API that has been deprecated for so long?

I see value in closing the gap for the differences (other commit) but implementing the show and hide (especially hide) I'm not so sure.

vince-fugnitto · 2022-05-24T19:15:53Z

@jfaltermeier @colin-grant-work if you still feel like the deprecated methods should in fact be supported then please disregard my comments :) I just thought it was a little odd to actually support the api given it was deprecated from the start.

colin-grant-work · 2022-05-24T19:24:26Z

I think I'm inclined to merge the implementation. It's strange that VSCode included deprecated API in their first public commit, and even stranger that they haven't removed it in the intervening 7 years, but as long as we've committed to supporting the VSCode plugin API, we should not reject a sound implementation, even of deprecated API.

vince-fugnitto · 2022-05-24T19:52:19Z

@jfaltermeier for the show editor beside command I noticed a difference between our behavior and vscode, it seems that vscode checks first if the requested editor already exists in that location and focuses it, rather than open a new editor:

theia:

vscode:

Steps:

open an editor
execute test show editor beside
focus original editor
re-do 2nd step

jfaltermeier · 2022-05-27T14:02:47Z

I'm on vacation until June 7th. I will pick this up again when I'm back.

jfaltermeier · 2022-06-07T14:19:41Z

This should work now. In case of ViewColumn.Beside we will now compute the actual ViewColumn index instead of falling back to split-right

vince-fugnitto

I confirm that the changes work as expected, I did not notice any regressions 👍

JonasHelming · 2022-06-09T14:09:48Z

@colin-grant-work : Fine if we merge this?

colin-grant-work · 2022-06-09T14:20:04Z

Ok with me. 👍

JonasHelming · 2022-06-09T14:23:19Z

@jfaltermeier : Please resolve conflict and merge ;-)

…a#11146 * extends theia.d.ts accordingly * adapt editor-manager to handle split editor open-request with missing counter option by computing the counter * add show/hide to TextEditorsMain interface Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com> Contributed on behalf of STMicroelectronics

* instead of falling back to split-right, compute the actual column index where the editor shall be shown * this will focus an existing editor for the file in the next tabbar if existing instead of always showing a new editor Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com> Contributed on behalf of STMicroelectronics

Ref: eclipse-theia/theia#11168 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

jfaltermeier added the vscode issues related to VSCode compatibility label May 17, 2022

jfaltermeier commented May 17, 2022

View reviewed changes

jfaltermeier force-pushed the 11146-texteditor-show-hide branch from a52d3bf to 0ea2088 Compare May 17, 2022 14:09

jfaltermeier marked this pull request as ready for review May 17, 2022 14:17

colin-grant-work reviewed May 17, 2022

View reviewed changes

jfaltermeier force-pushed the 11146-texteditor-show-hide branch 2 times, most recently from 0ea2088 to 44dab0c Compare May 18, 2022 09:09

jfaltermeier force-pushed the 11146-texteditor-show-hide branch from 44dab0c to 4ab2638 Compare May 19, 2022 07:26

vince-fugnitto reviewed May 19, 2022

View reviewed changes

jfaltermeier force-pushed the 11146-texteditor-show-hide branch from 432feed to 37dc356 Compare June 7, 2022 13:32

jfaltermeier requested a review from vince-fugnitto June 8, 2022 14:00

colin-grant-work self-requested a review June 8, 2022 20:47

vince-fugnitto reviewed Jun 9, 2022

View reviewed changes

vince-fugnitto approved these changes Jun 9, 2022

View reviewed changes

jfaltermeier and others added 3 commits June 14, 2022 08:34

More flexible editor and editor preview handling

e3c561e

jfaltermeier force-pushed the 11146-texteditor-show-hide branch from 37dc356 to 3e3f0af Compare June 14, 2022 06:34

jfaltermeier merged commit 0512a5f into eclipse-theia:master Jun 14, 2022

jfaltermeier deleted the 11146-texteditor-show-hide branch June 14, 2022 07:37

vince-fugnitto added this to the 1.27.0 milestone Jun 30, 2022

kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Jul 25, 2022

Workaround for duplicate editor tabs from Theia.

e9b471c

Ref: eclipse-theia/theia#11168 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos pushed a commit to kittaakos/arduino-ide that referenced this pull request Nov 8, 2022

Workaround for duplicate editor tabs from Theia.

a602cf7

Ref: eclipse-theia/theia#11168 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

kittaakos pushed a commit to kittaakos/arduino-ide that referenced this pull request Nov 10, 2022

Workaround for duplicate editor tabs from Theia.

023c27c

Ref: eclipse-theia/theia#11168 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vscode] Support TextEditor#show() and TextEditor#hide() #11146 #11168

[vscode] Support TextEditor#show() and TextEditor#hide() #11146 #11168

jfaltermeier commented May 17, 2022 •

edited

Loading

jfaltermeier May 17, 2022

colin-grant-work May 17, 2022

jfaltermeier commented May 17, 2022 •

edited

Loading

vince-fugnitto commented May 17, 2022

colin-grant-work May 17, 2022

jfaltermeier May 18, 2022

colin-grant-work May 17, 2022

colin-grant-work commented May 17, 2022 •

edited

Loading

colin-grant-work commented May 17, 2022 •

edited

Loading

jfaltermeier commented May 18, 2022

colin-grant-work commented May 18, 2022

vince-fugnitto left a comment

vince-fugnitto May 19, 2022

jfaltermeier May 23, 2022

jfaltermeier commented May 23, 2022

vince-fugnitto commented May 24, 2022 •

edited

Loading

vince-fugnitto commented May 24, 2022

colin-grant-work commented May 24, 2022

vince-fugnitto commented May 24, 2022

jfaltermeier commented May 27, 2022

jfaltermeier commented Jun 7, 2022

vince-fugnitto left a comment

JonasHelming commented Jun 9, 2022 •

edited

Loading

colin-grant-work commented Jun 9, 2022

JonasHelming commented Jun 9, 2022

[vscode] Support TextEditor#show() and TextEditor#hide() #11146 #11168

[vscode] Support TextEditor#show() and TextEditor#hide() #11146 #11168

Conversation

jfaltermeier commented May 17, 2022 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

jfaltermeier May 17, 2022

Choose a reason for hiding this comment

colin-grant-work May 17, 2022

Choose a reason for hiding this comment

jfaltermeier commented May 17, 2022 • edited Loading

vince-fugnitto commented May 17, 2022

colin-grant-work May 17, 2022

Choose a reason for hiding this comment

jfaltermeier May 18, 2022

Choose a reason for hiding this comment

colin-grant-work May 17, 2022

Choose a reason for hiding this comment

colin-grant-work commented May 17, 2022 • edited Loading

colin-grant-work commented May 17, 2022 • edited Loading

jfaltermeier commented May 18, 2022

colin-grant-work commented May 18, 2022

vince-fugnitto left a comment

Choose a reason for hiding this comment

vince-fugnitto May 19, 2022

Choose a reason for hiding this comment

jfaltermeier May 23, 2022

Choose a reason for hiding this comment

jfaltermeier commented May 23, 2022

vince-fugnitto commented May 24, 2022 • edited Loading

vince-fugnitto commented May 24, 2022

colin-grant-work commented May 24, 2022

vince-fugnitto commented May 24, 2022

Steps:

jfaltermeier commented May 27, 2022

jfaltermeier commented Jun 7, 2022

vince-fugnitto left a comment

Choose a reason for hiding this comment

JonasHelming commented Jun 9, 2022 • edited Loading

colin-grant-work commented Jun 9, 2022

JonasHelming commented Jun 9, 2022

jfaltermeier commented May 17, 2022 •

edited

Loading

jfaltermeier commented May 17, 2022 •

edited

Loading

colin-grant-work commented May 17, 2022 •

edited

Loading

colin-grant-work commented May 17, 2022 •

edited

Loading

vince-fugnitto commented May 24, 2022 •

edited

Loading

JonasHelming commented Jun 9, 2022 •

edited

Loading