Improve vscode tab API - Githubissues

eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.

http://theia-ide.org

Eclipse Public License 2.0

19.55k stars 2.46k forks source link

Improve vscode tab API #13730

Closed jonah-iden closed 1 month ago

jonah-iden commented 1 month ago

What it does

This improves and fixes some things for the vscode tab api.

Now there should always be a default tabGroup open, even when no tabs are open
Implements the viewCollumn property
Smaller fix for tab events being fired even though there was no change

How to test

Here is a small test extension. This shows message and logs into the console the different tabApi events.

Follow-ups

Review checklist

[x] 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

tsmaeder commented 1 month ago

@jonah-iden do we have a list of issues this PR fixes?

jonah-iden commented 1 month ago

I think its just the List above with these three points. Are you still seeing Problems or discrepancies somewhere?

jonah-iden commented 1 month ago

It can easily be that i overlooked problems. Its sadly super hard to align this perfectly to vscode because phosphor is just doing things differently to vscodes layouting. Like events just happening in different order

tsmaeder commented 1 month ago

https://github.com/eclipse-theia/theia/issues/12702 https://github.com/eclipse-theia/theia/issues/13674

Also, we should try to remove the workaround in the tests I introduced for the 1.88.1 built-ins. https://github.com/eclipse-theia/theia/issues/13679#issuecomment-2094653160

tsmaeder commented 1 month ago

Removing the workaround for https://github.com/eclipse-theia/theia/issues/13679 makes the tests fail again. Seems that one at least is still there.

msujew commented 1 month ago

@tsmaeder FYI the latest commit fixes an error in the plugin host that appeared due to an off-by-one error. Note that the typescript.isManagedFile context key seems to work unreliably and cannot be used to check whether the TypeScript language services are available (or not). See also https://github.com/eclipse-theia/theia/pull/13730#discussion_r1619148021. This could be potentially fixed in a separate PR. However it fixes the initial issue that causes https://github.com/eclipse-theia/theia/issues/13674, to this PR should be good to go.

tsmaeder commented 1 month ago

@msujew we should keep the issue reference, since adding the one second timeout is necessary as long as setting the context does not work reliably. Otherwise, this PR looks fine as far as it goes. Unfortunately, typescript-language-features has started to rely extensively on getting the correct active tab and tab group, so I think getting the behavior aligned with VS Code is kinda non-optional, eventually.

msujew commented 1 month ago

FYI I believe I've mostly fixed https://github.com/eclipse-theia/theia/issues/13679, it's just a minor issue left: The onDidChangeActiveTextEditor event is fired on the plugin host before the tab API has been updated on the plugin host (due to the order of events in the frontend). However, I couldn't find a quick way to fix this without breaking a bunch of other stuff, so I'm putting this off for a separate PR.

The original off-by-one issue mentioned in the issue is fixed, it's just a matter of correctly ordering the events.