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

Improve vscode tab API #13730

Merged
merged 8 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/api-tests/src/typescript.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('TypeScript', function () {
const editorWidget = widget instanceof EditorWidget ? widget : undefined;
const editor = MonacoEditor.get(editorWidget);
assert.isDefined(editor);
await timeout(1000); // workaround for https://github.com/eclipse-theia/theia/issues/13679
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the workaround: the workaround is not waiting for the context key typescrip.isManagedFile.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I need the 1 second timeout for the tests to run reliably, locally.

Copy link
Member

Choose a reason for hiding this comment

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

The typescript.isManagedFile context key is working unreliably. When opening a TypeScript file in the editor and querying the context key value, I always get false, even though a TypeScript editor is open (and focused).

I've reinstated the 1 second timeout, as the issue seems to be unrelated to the tab API.

await timeout(1000);
msujew marked this conversation as resolved.
Show resolved Hide resolved
// wait till tsserver is running, see:
// https://github.com/microsoft/vscode/blob/93cbbc5cae50e9f5f5046343c751b6d010468200/extensions/typescript-language-features/src/extension.ts#L98-L103
// await waitForAnimation(() => contextKeyService.match('typescript.isManagedFile'));
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/browser/frontend-application-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ import { AdditionalViewsMenuWidget, AdditionalViewsMenuWidgetFactory } from './s
import { LanguageIconLabelProvider } from './language-icon-provider';
import { bindTreePreferences } from './tree';
import { OpenWithService } from './open-with-service';
import { ViewColumnService } from './shell/view-column-service';

export { bindResourceProvider, bindMessageService, bindPreferenceService };

Expand Down Expand Up @@ -462,4 +463,6 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
bind(FrontendApplicationContribution).toService(StylingService);

bind(SecondaryWindowHandler).toSelf().inSingletonScope();

bind(ViewColumnService).toSelf().inSingletonScope();
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { injectable, inject } from '@theia/core/shared/inversify';
import { Emitter, Event } from '@theia/core/lib/common/event';
import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
import { toArray } from '@theia/core/shared/@phosphor/algorithm';
import { TabBar, Widget } from '@theia/core/shared/@phosphor/widgets';
import { injectable, inject } from 'inversify';
import { Emitter, Event } from '../../common/event';
import { ApplicationShell } from './application-shell';
import { toArray } from '@phosphor/algorithm';
import { TabBar, Widget } from '@phosphor/widgets';

@injectable()
export class ViewColumnService {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { PluginDebugService } from './debug/plugin-debug-service';
import { DebugService } from '@theia/debug/lib/common/debug-service';
import { PluginSharedStyle } from './plugin-shared-style';
import { SelectionProviderCommandContribution } from './selection-provider-command';
import { ViewColumnService } from './view-column-service';
import { ViewContextKeyService } from './view/view-context-key-service';
import { PluginViewWidget, PluginViewWidgetIdentifier } from './view/plugin-view-widget';
import { TreeViewWidgetOptions, VIEW_ITEM_CONTEXT_MENU, PluginTree, TreeViewWidget, PluginTreeModel } from './view/tree-view-widget';
Expand Down Expand Up @@ -244,8 +243,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
bind(PluginDebugSessionContributionRegistry).toSelf().inSingletonScope();
rebind(DebugSessionContributionRegistry).toService(PluginDebugSessionContributionRegistry);

bind(ViewColumnService).toSelf().inSingletonScope();

bind(CommentsService).to(PluginCommentService).inSingletonScope();
bind(CommentingRangeDecorator).toSelf().inSingletonScope();
bind(CommentsContribution).toSelf().inSingletonScope();
Expand Down
45 changes: 36 additions & 9 deletions packages/plugin-ext/src/main/browser/tabs/tabs-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { MonacoDiffEditor } from '@theia/monaco/lib/browser/monaco-diff-editor';
import { toUriComponents } from '../hierarchy/hierarchy-types-converters';
import { TerminalWidget } from '@theia/terminal/lib/browser/base/terminal-widget';
import { DisposableCollection } from '@theia/core';
import { NotebookEditorWidget } from '@theia/notebook/lib/browser';
import { ViewColumnService } from '@theia/core/lib/browser/shell/view-column-service';

interface TabInfo {
tab: TabDto;
Expand All @@ -46,6 +48,14 @@ export class TabsMainImpl implements TabsMain, Disposable {
private currentActiveGroup: TabGroupDto;

private tabGroupChanged: boolean = false;
private viewColumnService: ViewColumnService;

private readonly defaultTabGroup: TabGroupDto = {
groupId: 0,
tabs: [],
isActive: true,
viewColumn: 0
};

constructor(
rpc: RPCProtocol,
Expand All @@ -54,6 +64,7 @@ export class TabsMainImpl implements TabsMain, Disposable {
this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.TABS_EXT);

this.applicationShell = container.get(ApplicationShell);
this.viewColumnService = container.get(ViewColumnService);
this.createTabsModel();

const tabBars = this.applicationShell.mainPanel.tabBars();
Expand Down Expand Up @@ -100,15 +111,20 @@ export class TabsMainImpl implements TabsMain, Disposable {
}

protected createTabsModel(): void {
if (this.applicationShell.mainAreaTabBars.length === 0) {
this.proxy.$acceptEditorTabModel([this.defaultTabGroup]);
return;
}
const newTabGroupModel = new Map<TabBar<Widget>, TabGroupDto>();
this.tabInfoLookup.clear();
this.disposableTabBarListeners.dispose();
this.applicationShell.mainAreaTabBars.forEach(tabBar => {
this.attachListenersToTabBar(tabBar);
const groupDto = this.createTabGroupDto(tabBar);
tabBar.titles.forEach((title, index) => this.tabInfoLookup.set(title, { group: groupDto, tab: groupDto.tabs[index], tabIndex: index }));
newTabGroupModel.set(tabBar, groupDto);
});
this.applicationShell.mainAreaTabBars
.forEach((tabBar, i) => {
this.attachListenersToTabBar(tabBar);
const groupDto = this.createTabGroupDto(tabBar, i);
tabBar.titles.forEach((title, index) => this.tabInfoLookup.set(title, { group: groupDto, tab: groupDto.tabs[index], tabIndex: index }));
newTabGroupModel.set(tabBar, groupDto);
});
if (newTabGroupModel.size > 0 && Array.from(newTabGroupModel.values()).indexOf(this.currentActiveGroup) < 0) {
this.currentActiveGroup = this.tabInfoLookup.get(this.applicationShell.mainPanel.currentTitle!)?.group ?? newTabGroupModel.values().next().value;
this.currentActiveGroup.isActive = true;
Expand All @@ -134,15 +150,17 @@ export class TabsMainImpl implements TabsMain, Disposable {
return `${groupId}~${tabTitle.owner.id}`;
}

protected createTabGroupDto(tabBar: TabBar<Widget>): TabGroupDto {
const oldDto = this.tabGroupModel.get(tabBar);
protected createTabGroupDto(tabBar: TabBar<Widget>, index: number): TabGroupDto {
const oldDto = index === 0 ? this.defaultTabGroup : this.tabGroupModel.get(tabBar);
const groupId = oldDto?.groupId ?? this.groupIdCounter++;
const tabs = tabBar.titles.map(title => this.createTabDto(title, groupId));
const viewColumn = this.viewColumnService.getViewColumn(tabBar.id)
?? this.applicationShell.allTabBars.indexOf(tabBar);
return {
groupId,
tabs,
isActive: false,
viewColumn: 1
viewColumn
};
}

Expand Down Expand Up @@ -182,6 +200,12 @@ export class TabsMainImpl implements TabsMain, Disposable {
return {
kind: TabInputKind.TerminalEditorInput
};
} else if (widget instanceof NotebookEditorWidget) {
return {
kind: TabInputKind.NotebookInput,
notebookType: widget.notebookType,
uri: toUriComponents(widget.model?.uri.toString() ?? '')
};
}

return { kind: TabInputKind.UnknownInput };
Expand Down Expand Up @@ -232,6 +256,9 @@ export class TabsMainImpl implements TabsMain, Disposable {
}
const oldTabDto = tabInfo.tab;
const newTabDto = this.createTabDto(title, tabInfo.group.groupId);
if (!oldTabDto.isActive && newTabDto.isActive) {
this.currentActiveGroup.tabs.filter(tab => tab.isActive).forEach(tab => tab.isActive = false);
}
if (newTabDto.isActive && !tabInfo.group.isActive) {
tabInfo.group.isActive = true;
this.currentActiveGroup.isActive = false;
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-ext/src/main/browser/webviews-main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { ViewBadge, WebviewOptions, WebviewPanelOptions, WebviewPanelShowOptions
import { ApplicationShell } from '@theia/core/lib/browser/shell/application-shell';
import { WebviewWidget, WebviewWidgetIdentifier } from './webview/webview';
import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable';
import { ViewColumnService } from './view-column-service';
import { ViewColumnService } from '@theia/core/lib/browser/shell/view-column-service';
import { WidgetManager } from '@theia/core/lib/browser/widget-manager';
import { JSONExt } from '@theia/core/shared/@phosphor/coreutils';
import { Mutable } from '@theia/core/lib/common/types';
Expand Down
9 changes: 7 additions & 2 deletions packages/plugin-ext/src/plugin/tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,13 @@ export class TabsExtImpl implements TabsExt {
this.activeGroupId = activeTabGroupId;
}
}
this.onDidChangeTabGroups.fire(Object.freeze({ opened, closed, changed }));
this.onDidChangeTabs.fire({ opened: tabsOpened, changed: [], closed: [] });

if (closed.length > 0 || opened.length > 0 || changed.length > 0) {
this.onDidChangeTabGroups.fire(Object.freeze({ opened, closed, changed }));
}
if (tabsOpened.length > 0) {
this.onDidChangeTabs.fire({ opened: tabsOpened, changed: [], closed: [] });
}
}

$acceptTabGroupUpdate(groupDto: TabGroupDto): void {
Expand Down
Loading