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

fixed #12392. Tab indices now updated #12400

Merged

Conversation

jonah-iden
Copy link
Contributor

after deleting and moving a tab

What it does

Fixes #12392
updates tabs indeces of all tabs in a group when a tab is deleted or moved

How to test

as described in the issue

  1. Open 4 editors
  2. Close the second-to-last (third) tab
  3. Close the last tab
  4. no execption should be thrown anymore

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 still notice errors with this approach:

rpc-message-encoder.ts:184 Uncaught (in promise) Error: Tab close updated received for index 3 which does not exist
    at TabGroupExt.acceptTabOperation (/workspace/theia/packages/plugin-ext/lib/plugin/tabs.js:156:23)
    at TabsExtImpl.$acceptTabOperation (/workspace/theia/packages/plugin-ext/lib/plugin/tabs.js:286:27)
    at /workspace/theia/packages/plugin-ext/lib/common/proxy-handler.js:91:71
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async RpcProtocol.handleRequest (/workspace/theia/packages/core/lib/common/message-rpc/rpc-protocol.js:167:28)

Steps:

  1. open 4 editors
  2. close the third editor (2nd last)
  3. close the last editor

@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Apr 12, 2023
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I can't seem to reproduce the error any longer 👍

packages/plugin-ext/src/main/browser/tabs/tabs-main.ts Outdated Show resolved Hide resolved
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 can confirm that the error is no longer reported in the console 👍

@msujew msujew force-pushed the jiden/tabs-delete-index-fix branch from 9b20e39 to 964a1ab Compare April 21, 2023 19:12
@vince-fugnitto vince-fugnitto force-pushed the jiden/tabs-delete-index-fix branch 2 times, most recently from b53e89d to b0fff47 Compare April 27, 2023 12:36
@vince-fugnitto vince-fugnitto modified the milestone: 1.37.0 Apr 27, 2023
@tsmaeder
Copy link
Contributor

tsmaeder commented May 8, 2023

@vince-fugnitto merge this one?

jonah-iden and others added 4 commits May 8, 2023 08:47
after deleting and moving a tab

Signed-off-by: Jonah Iden <jonah.iden@typefox.io>
Signed-off-by: Jonah Iden <jonah.iden@typefox.io>
Co-authored-by: Mark Sujew <mark.sujew@typefox.io>
@vince-fugnitto vince-fugnitto merged commit cc30170 into eclipse-theia:master May 8, 2023
@vince-fugnitto vince-fugnitto added this to the 1.38.0 milestone Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

TabAPI throws Error when Closing
4 participants