Skip to content

Commit

Permalink
Fix #15178 Finalize tabs API 🎉
Browse files Browse the repository at this point in the history
  • Loading branch information
@lramos15
lramos15 committed Apr 21, 2022
1 parent 2b3212e commit aa69f3d
Show file tree
Hide file tree
Showing 9 changed files with 369 additions and 364 deletions.
10 changes: 5 additions & 5 deletions extensions/git/src/repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import * as fs from 'fs';
import * as path from 'path';
import { CancellationToken, Command, Disposable, Event, EventEmitter, Memento, OutputChannel, ProgressLocation, ProgressOptions, scm, SourceControl, SourceControlInputBox, SourceControlInputBoxValidation, SourceControlInputBoxValidationType, SourceControlResourceDecorations, SourceControlResourceGroup, SourceControlResourceState, ThemeColor, Uri, window, workspace, WorkspaceEdit, FileDecoration, commands, Tab, TabKindTextDiff, TabKindNotebookDiff, RelativePattern } from 'vscode';
import { CancellationToken, Command, Disposable, Event, EventEmitter, Memento, OutputChannel, ProgressLocation, ProgressOptions, scm, SourceControl, SourceControlInputBox, SourceControlInputBoxValidation, SourceControlInputBoxValidationType, SourceControlResourceDecorations, SourceControlResourceGroup, SourceControlResourceState, ThemeColor, Uri, window, workspace, WorkspaceEdit, FileDecoration, commands, Tab, TabInputTextDiff, TabInputNotebookDiff, RelativePattern } from 'vscode';
import TelemetryReporter from '@vscode/extension-telemetry';
import * as nls from 'vscode-nls';
import { Branch, Change, ForcePushMode, GitErrorCodes, LogOptions, Ref, RefType, Remote, Status, CommitOptions, BranchQuery, FetchOptions } from './api/git';
Expand Down Expand Up @@ -1274,13 +1274,13 @@ export class Repository implements Disposable {
const diffEditorTabsToClose: Tab[] = [];

for (const tab of window.tabGroups.all.map(g => g.tabs).flat()) {
const { kind } = tab;
if (kind instanceof TabKindTextDiff || kind instanceof TabKindNotebookDiff) {
if (kind.modified.scheme === 'git' && indexResources.some(r => pathEquals(r, kind.modified.fsPath))) {
const { input } = tab;
if (input instanceof TabInputTextDiff || input instanceof TabInputNotebookDiff) {
if (input.modified.scheme === 'git' && indexResources.some(r => pathEquals(r, input.modified.fsPath))) {
// Index
diffEditorTabsToClose.push(tab);
}
if (kind.modified.scheme === 'file' && kind.original.scheme === 'git' && workingTreeResources.some(r => pathEquals(r, kind.modified.fsPath))) {
if (input.modified.scheme === 'file' && input.original.scheme === 'git' && workingTreeResources.some(r => pathEquals(r, input.modified.fsPath))) {
// Working Tree
diffEditorTabsToClose.push(tab);
}
Expand Down
84 changes: 42 additions & 42 deletions extensions/vscode-api-tests/src/singlefolder-tests/window.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import * as assert from 'assert';
import { join } from 'path';
import { CancellationTokenSource, commands, MarkdownString, TabKindNotebook, Position, QuickPickItem, Selection, StatusBarAlignment, TabKindTextDiff, TextEditor, TextEditorSelectionChangeKind, TextEditorViewColumnChangeEvent, TabKindText, Uri, ViewColumn, window, workspace } from 'vscode';
import { CancellationTokenSource, commands, MarkdownString, TabInputNotebook, Position, QuickPickItem, Selection, StatusBarAlignment, TextEditor, TextEditorSelectionChangeKind, TextEditorViewColumnChangeEvent, TabInputText, Uri, ViewColumn, window, workspace, TabInputTextDiff } from 'vscode';
import { assertNoRpc, closeAllEditors, createRandomFile, pathEquals } from '../utils';


Expand Down Expand Up @@ -371,28 +371,28 @@ suite('vscode API - window', () => {
});

//#region Tabs API tests
test('Tabs - move tab', async function () {
const [docA, docB, docC] = await Promise.all([
workspace.openTextDocument(await createRandomFile()),
workspace.openTextDocument(await createRandomFile()),
workspace.openTextDocument(await createRandomFile())
]);
// test('Tabs - move tab', async function () {
// const [docA, docB, docC] = await Promise.all([
// workspace.openTextDocument(await createRandomFile()),
// workspace.openTextDocument(await createRandomFile()),
// workspace.openTextDocument(await createRandomFile())
// ]);

await window.showTextDocument(docA, { viewColumn: ViewColumn.One, preview: false });
await window.showTextDocument(docB, { viewColumn: ViewColumn.One, preview: false });
await window.showTextDocument(docC, { viewColumn: ViewColumn.Two, preview: false });
// await window.showTextDocument(docA, { viewColumn: ViewColumn.One, preview: false });
// await window.showTextDocument(docB, { viewColumn: ViewColumn.One, preview: false });
// await window.showTextDocument(docC, { viewColumn: ViewColumn.Two, preview: false });

const tabGroups = window.tabGroups;
assert.strictEqual(tabGroups.all.length, 2);
// const tabGroups = window.tabGroups;
// assert.strictEqual(tabGroups.all.length, 2);

const group1Tabs = tabGroups.all[0].tabs;
assert.strictEqual(group1Tabs.length, 2);
// const group1Tabs = tabGroups.all[0].tabs;
// assert.strictEqual(group1Tabs.length, 2);

const group2Tabs = tabGroups.all[1].tabs;
assert.strictEqual(group2Tabs.length, 1);
// const group2Tabs = tabGroups.all[1].tabs;
// assert.strictEqual(group2Tabs.length, 1);

await tabGroups.move(group1Tabs[0], ViewColumn.One, 1);
});
// await tabGroups.move(group1Tabs[0], ViewColumn.One, 1);
// });

// TODO @lramos15 re-enable these once shape is more stable
test('Tabs - vscode.open & vscode.diff', async function () {
Expand Down Expand Up @@ -423,14 +423,14 @@ suite('vscode API - window', () => {

const tabs = window.tabGroups.all.map(g => g.tabs).flat(1);
assert.strictEqual(tabs.length, 5);
assert.ok(tabs[0].kind instanceof TabKindText);
assert.strictEqual(tabs[0].kind.uri.toString(), docA.uri.toString());
assert.ok(tabs[1].kind instanceof TabKindText);
assert.strictEqual(tabs[1].kind.uri.toString(), docB.uri.toString());
assert.ok(tabs[2].kind instanceof TabKindText);
assert.strictEqual(tabs[2].kind.uri.toString(), docC.uri.toString());
assert.ok(tabs[3].kind instanceof TabKindText);
assert.strictEqual(tabs[3].kind.uri.toString(), commandFile.toString());
assert.ok(tabs[0].input instanceof TabInputText);
assert.strictEqual(tabs[0].input.uri.toString(), docA.uri.toString());
assert.ok(tabs[1].input instanceof TabInputText);
assert.strictEqual(tabs[1].input.uri.toString(), docB.uri.toString());
assert.ok(tabs[2].input instanceof TabInputText);
assert.strictEqual(tabs[2].input.uri.toString(), docC.uri.toString());
assert.ok(tabs[3].input instanceof TabInputText);
assert.strictEqual(tabs[3].input.uri.toString(), commandFile.toString());
});

test('Tabs - Ensure tabs getter is correct', async function () {
Expand Down Expand Up @@ -459,17 +459,17 @@ suite('vscode API - window', () => {
assert.strictEqual(tabs.length, 5);

// All resources should match the text documents as they're the only tabs currently open
assert.ok(tabs[0].kind instanceof TabKindText);
assert.strictEqual(tabs[0].kind.uri.toString(), docA.uri.toString());
assert.ok(tabs[1].kind instanceof TabKindNotebook);
assert.strictEqual(tabs[1].kind.uri.toString(), notebookDoc.uri.toString());
assert.ok(tabs[2].kind instanceof TabKindText);
assert.strictEqual(tabs[2].kind.uri.toString(), docB.uri.toString());
assert.ok(tabs[3].kind instanceof TabKindText);
assert.strictEqual(tabs[3].kind.uri.toString(), docC.uri.toString());
assert.ok(tabs[0].input instanceof TabInputText);
assert.strictEqual(tabs[0].input.uri.toString(), docA.uri.toString());
assert.ok(tabs[1].input instanceof TabInputNotebook);
assert.strictEqual(tabs[1].input.uri.toString(), notebookDoc.uri.toString());
assert.ok(tabs[2].input instanceof TabInputText);
assert.strictEqual(tabs[2].input.uri.toString(), docB.uri.toString());
assert.ok(tabs[3].input instanceof TabInputText);
assert.strictEqual(tabs[3].input.uri.toString(), docC.uri.toString());
// Diff editor and side by side editor report the right side as the resource
assert.ok(tabs[4].kind instanceof TabKindTextDiff);
assert.strictEqual(tabs[4].kind.modified.toString(), rightDiff.toString());
assert.ok(tabs[4].input instanceof TabInputTextDiff);
assert.strictEqual(tabs[4].input.modified.toString(), rightDiff.toString());

assert.strictEqual(tabs[0].group.viewColumn, ViewColumn.One);
assert.strictEqual(tabs[1].group.viewColumn, ViewColumn.One);
Expand All @@ -495,20 +495,20 @@ suite('vscode API - window', () => {
await window.showTextDocument(docA, { viewColumn: ViewColumn.One, preview: false });
let activeTab = getActiveTabInActiveGroup();
assert.ok(activeTab);
assert.ok(activeTab.kind instanceof TabKindText);
assert.strictEqual(activeTab.kind.uri.toString(), docA.uri.toString());
assert.ok(activeTab.input instanceof TabInputText);
assert.strictEqual(activeTab.input.uri.toString(), docA.uri.toString());

await window.showTextDocument(docB, { viewColumn: ViewColumn.Two, preview: false });
activeTab = getActiveTabInActiveGroup();
assert.ok(activeTab);
assert.ok(activeTab.kind instanceof TabKindText);
assert.strictEqual(activeTab.kind.uri.toString(), docB.uri.toString());
assert.ok(activeTab.input instanceof TabInputText);
assert.strictEqual(activeTab.input.uri.toString(), docB.uri.toString());

await window.showTextDocument(docC, { viewColumn: ViewColumn.Three, preview: false });
activeTab = getActiveTabInActiveGroup();
assert.ok(activeTab);
assert.ok(activeTab.kind instanceof TabKindText);
assert.strictEqual(activeTab.kind.uri.toString(), docC.uri.toString());
assert.ok(activeTab.input instanceof TabInputText);
assert.strictEqual(activeTab.input.uri.toString(), docC.uri.toString());

await commands.executeCommand('workbench.action.closeActiveEditor');
await commands.executeCommand('workbench.action.closeActiveEditor');
Expand Down
15 changes: 7 additions & 8 deletions src/vs/workbench/api/common/extHost.api.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
return extHostUriOpeners.registerExternalUriOpener(extension.identifier, id, opener, metadata);
},
get tabGroups(): vscode.TabGroups {
checkProposedApiEnabled(extension, 'tabs');
return extHostEditorTabs.tabGroups;
},
getInlineCompletionItemController<T extends vscode.InlineCompletionItem>(provider: vscode.InlineCompletionItemProvider<T>): vscode.InlineCompletionController<T> {
Expand Down Expand Up @@ -1345,13 +1344,13 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
LanguageStatusSeverity: extHostTypes.LanguageStatusSeverity,
QuickPickItemKind: extHostTypes.QuickPickItemKind,
InputBoxValidationSeverity: extHostTypes.InputBoxValidationSeverity,
TabKindText: extHostTypes.TextTabInput,
TabKindTextDiff: extHostTypes.TextDiffTabInput,
TabKindCustom: extHostTypes.CustomEditorTabInput,
TabKindNotebook: extHostTypes.NotebookEditorTabInput,
TabKindNotebookDiff: extHostTypes.NotebookDiffEditorTabInput,
TabKindWebview: extHostTypes.WebviewEditorTabInput,
TabKindTerminal: extHostTypes.TerminalEditorTabInput
TabInputText: extHostTypes.TextTabInput,
TabInputTextDiff: extHostTypes.TextDiffTabInput,
TabInputCustom: extHostTypes.CustomEditorTabInput,
TabInputNotebook: extHostTypes.NotebookEditorTabInput,
TabInputNotebookDiff: extHostTypes.NotebookDiffEditorTabInput,
TabInputWebview: extHostTypes.WebviewEditorTabInput,
TabInputTerminal: extHostTypes.TerminalEditorTabInput
};
};
}
20 changes: 10 additions & 10 deletions src/vs/workbench/api/common/extHostEditorTabs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IEditorTabDto, IEditorTabGroupDto, IExtHostEditorTabsShape, MainContext
import { URI } from 'vs/base/common/uri';
import { Emitter } from 'vs/base/common/event';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { CustomEditorTabInput, NotebookDiffEditorTabInput, NotebookEditorTabInput, TerminalEditorTabInput, TextDiffTabInput, TextTabInput, ViewColumn, WebviewEditorTabInput } from 'vs/workbench/api/common/extHostTypes';
import { CustomEditorTabInput, NotebookDiffEditorTabInput, NotebookEditorTabInput, TerminalEditorTabInput, TextDiffTabInput, TextTabInput, WebviewEditorTabInput } from 'vs/workbench/api/common/extHostTypes';
import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
import { assertIsDefined } from 'vs/base/common/types';
import { diffSets } from 'vs/base/common/collections';
Expand Down Expand Up @@ -48,7 +48,7 @@ class ExtHostEditorTab {
get label() {
return that._dto.label;
},
get kind() {
get input() {
return that._input;
},
get isDirty() {
Expand Down Expand Up @@ -256,14 +256,14 @@ export class ExtHostEditorTabs implements IExtHostEditorTabs {
return this._closeTabs(tabsOrTabGroups as vscode.Tab[], preserveFocus);
}
},
move: async (tab: vscode.Tab, viewColumn: ViewColumn, index: number, preservceFocus?: boolean) => {
const extHostTab = this._findExtHostTabFromApi(tab);
if (!extHostTab) {
throw new Error('Invalid tab');
}
this._proxy.$moveTab(extHostTab.tabId, index, typeConverters.ViewColumn.from(viewColumn), preservceFocus);
return;
}
// move: async (tab: vscode.Tab, viewColumn: ViewColumn, index: number, preservceFocus?: boolean) => {
// const extHostTab = this._findExtHostTabFromApi(tab);
// if (!extHostTab) {
// throw new Error('Invalid tab');
// }
// this._proxy.$moveTab(extHostTab.tabId, index, typeConverters.ViewColumn.from(viewColumn), preservceFocus);
// return;
// }
};
this._apiObject = Object.freeze(obj);
}
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/api/common/extHostVariableResolverService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ class ExtHostVariableResolverService extends AbstractVariableResolverService {
const activeTab = editorTabs.tabGroups.all.find(group => group.isActive)?.activeTab;
if (activeTab !== undefined) {
// Resolve a resource from the tab
if (activeTab.kind instanceof TextDiffTabInput || activeTab.kind instanceof NotebookDiffEditorTabInput) {
return activeTab.kind.modified;
} else if (activeTab.kind instanceof TextTabInput || activeTab.kind instanceof NotebookEditorTabInput || activeTab.kind instanceof CustomEditorTabInput) {
return activeTab.kind.uri;
if (activeTab.input instanceof TextDiffTabInput || activeTab.input instanceof NotebookDiffEditorTabInput) {
return activeTab.input.modified;
} else if (activeTab.input instanceof TextTabInput || activeTab.input instanceof NotebookEditorTabInput || activeTab.input instanceof CustomEditorTabInput) {
return activeTab.input.uri;
}
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/vs/workbench/api/test/browser/extHostEditorTabs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,10 @@ suite('ExtHostEditorTabs', function () {
let all = extHostEditorTabs.tabGroups.all.map(group => group.tabs).flat();
assert.strictEqual(all.length, 1);
const apiTab1 = all[0];
assert.ok(apiTab1.kind instanceof TextTabInput);
assert.ok(apiTab1.input instanceof TextTabInput);
assert.strictEqual(tabDto.input.kind, TabInputKind.TextInput);
const dtoResource = (tabDto.input as TextInputDto).uri;
assert.strictEqual(apiTab1.kind.uri.toString(), URI.revive(dtoResource).toString());
assert.strictEqual(apiTab1.input.uri.toString(), URI.revive(dtoResource).toString());
assert.strictEqual(apiTab1.isDirty, true);


Expand All @@ -250,8 +250,8 @@ suite('ExtHostEditorTabs', function () {
all = extHostEditorTabs.tabGroups.all.map(group => group.tabs).flat();
assert.strictEqual(all.length, 1);
const apiTab2 = all[0];
assert.ok(apiTab1.kind instanceof TextTabInput);
assert.strictEqual(apiTab1.kind.uri.toString(), URI.revive(dtoResource).toString());
assert.ok(apiTab1.input instanceof TextTabInput);
assert.strictEqual(apiTab1.input.uri.toString(), URI.revive(dtoResource).toString());
assert.strictEqual(apiTab2.isDirty, false);

assert.strictEqual(apiTab1 === apiTab2, true);
Expand Down Expand Up @@ -297,10 +297,10 @@ suite('ExtHostEditorTabs', function () {
assert.strictEqual(all.length, 2);

const activeTab1 = extHostEditorTabs.tabGroups.activeTabGroup?.activeTab;
assert.ok(activeTab1?.kind instanceof TextTabInput);
assert.ok(activeTab1?.input instanceof TextTabInput);
assert.strictEqual(tabDtoAAA.input.kind, TabInputKind.TextInput);
const dtoAAAResource = (tabDtoAAA.input as TextInputDto).uri;
assert.strictEqual(activeTab1?.kind?.uri.toString(), URI.revive(dtoAAAResource)?.toString());
assert.strictEqual(activeTab1?.input?.uri.toString(), URI.revive(dtoAAAResource)?.toString());
assert.strictEqual(activeTab1?.isActive, true);

extHostEditorTabs.$acceptTabOperation({
Expand All @@ -311,10 +311,10 @@ suite('ExtHostEditorTabs', function () {
});

const activeTab2 = extHostEditorTabs.tabGroups.activeTabGroup?.activeTab;
assert.ok(activeTab2?.kind instanceof TextTabInput);
assert.ok(activeTab2?.input instanceof TextTabInput);
assert.strictEqual(tabDtoBBB.input.kind, TabInputKind.TextInput);
const dtoBBBResource = (tabDtoBBB.input as TextInputDto).uri;
assert.strictEqual(activeTab2?.kind?.uri.toString(), URI.revive(dtoBBBResource)?.toString());
assert.strictEqual(activeTab2?.input?.uri.toString(), URI.revive(dtoBBBResource)?.toString());
assert.strictEqual(activeTab2?.isActive, true);
assert.strictEqual(activeTab1?.isActive, false);
});
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/services/extensions/common/extensionsApiProposals.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export const allApiProposals = Object.freeze({
scmActionButton: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.scmActionButton.d.ts',
scmSelectedProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.scmSelectedProvider.d.ts',
scmValidation: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.scmValidation.d.ts',
tabs: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.tabs.d.ts',
taskPresentationGroup: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.taskPresentationGroup.d.ts',
telemetry: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.telemetry.d.ts',
terminalDataWriteEvent: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.terminalDataWriteEvent.d.ts',
Expand Down
Loading

2 comments on commit aa69f3d

@ArturoDent
Copy link

@ArturoDent ArturoDent commented on aa69f3d Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like the move function was removed. It seemed to be working well.

Is the expected alternative to use the moveActiveEditor command, see #8234 (comment) ?

It looks like it would be awkward to move both a tab from one group to another and change its position in that new group using moveActiveEditor - requiring 2 calls: (1) move the tab to the new group and (2) move it to the desired position in that new group.

Is there a better option?

@lramos15
Copy link
Member Author

Choose a reason for hiding this comment

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

@ArturoDent Unfortunately the move function did not fit the current set of functions that we wanted to finalize this iteration and had some further open question we wanted to polish in another iteration of the API focusing on modification. For example if I move to a new group how do I dictate the split direction of the group? How do I move groups as a whole? Can I save and restore a layout? Close was left as it doesn't really relate to the view state. For now the linked command is your best alternative.

Please sign in to comment.