From 1fedd4b93255be6e03d4be114f375cb53e56d38a Mon Sep 17 00:00:00 2001 From: Logan Ramos Date: Thu, 7 Apr 2022 09:27:11 -0400 Subject: [PATCH] Address PR feedback --- tabs-api-sample/package.json | 3 +- tabs-api-sample/src/extension.ts | 27 +++-- tabs-api-sample/vscode.d.ts | 43 +++++--- tabs-api-sample/vscode.proposed.tabs.d.ts | 128 ++++++++++++++++------ 4 files changed, 142 insertions(+), 59 deletions(-) diff --git a/tabs-api-sample/package.json b/tabs-api-sample/package.json index a0277049..aebe3ce4 100644 --- a/tabs-api-sample/package.json +++ b/tabs-api-sample/package.json @@ -15,9 +15,10 @@ "Other" ], "activationEvents": [ - "*" + "onStartupFinished" ], "main": "./out/extension.js", + "browser": "./out/extension.js", "contributes": { "configuration":{ "title": "Tabs API Sample", diff --git a/tabs-api-sample/src/extension.ts b/tabs-api-sample/src/extension.ts index a5837d83..cb0a0317 100644 --- a/tabs-api-sample/src/extension.ts +++ b/tabs-api-sample/src/extension.ts @@ -8,22 +8,27 @@ export function activate(context: vscode.ExtensionContext) { * Proposed API as defined in vscode.proposed..d.ts. */ const activityMap: Map = new Map(); - const disposables: vscode.Disposable[] = []; - disposables.push(vscode.window.tabGroups.onDidChangeTabGroups(() => { - const tabs = vscode.window.tabGroups.groups.map(group => group.tabs).flat(1); + context.subscriptions.push(vscode.window.tabGroups.onDidChangeTabGroups(() => { + const tabs = vscode.window.tabGroups.all.map(group => group.tabs).flat(1); activityMap.clear(); // Update activity map saying everything is active tabs.forEach(t => activityMap.set(t, Date.now())); })); - disposables.push(vscode.window.tabGroups.onDidChangeTabs(tabs => { - for (const tab of tabs) { + context.subscriptions.push(vscode.window.tabGroups.onDidChangeTabs(tabChangeEvent => { + // If tabs are closed no longer track their activity + for (const removedTab of tabChangeEvent.removed) { + activityMap.delete(removedTab); + } + // Update activity map last active time on changed tabs + const changedTabs = [...tabChangeEvent.added, ...tabChangeEvent.changed]; + for (const tab of changedTabs) { // Reset the timer for the tabs last activity activityMap.set(tab, Date.now()); } })); // Check every second for inactive tabs - setInterval(() => { + const interval = setInterval(() => { const activeTab = vscode.window.tabGroups.activeTabGroup.activeTab; if (activeTab) { // Update the activity map for the active tab, since it's still being used. @@ -31,12 +36,18 @@ export function activate(context: vscode.ExtensionContext) { } const inactiveTime = (vscode.workspace.getConfiguration().get('tabs.maxInactiveTime') ?? 30) * 1000; // Check if any tabs have been inactive for more than 30 seconds - const inactiveTabs = Array.from(activityMap.entries()).filter(([tab, lastActive]) => Date.now() - lastActive > inactiveTime && vscode.window.tabGroups.activeTabGroup.activeTab !== tab && !tab.isDirty); + const inactiveTabs = Array.from(activityMap.entries()).filter(([tab, lastActive]) => Date.now() - lastActive > inactiveTime && !tab.isActive && !tab.isDirty); inactiveTabs.forEach(async ([tab]) => { // Close the tab await vscode.window.tabGroups.close(tab); activityMap.delete(tab); }); }, 1000); - disposables.forEach(d => context.subscriptions.push(d)); + + // Create a small disposable to clean up the interval when extension is deactivated + context.subscriptions.push({ + dispose: () => { + clearInterval(interval); + } + }); } diff --git a/tabs-api-sample/vscode.d.ts b/tabs-api-sample/vscode.d.ts index 34c0b1e1..02c23295 100644 --- a/tabs-api-sample/vscode.d.ts +++ b/tabs-api-sample/vscode.d.ts @@ -3530,7 +3530,7 @@ declare module 'vscode' { * value starting at 1. * @return This snippet string. */ - appendChoice(values: string[], number?: number): SnippetString; + appendChoice(values: readonly string[], number?: number): SnippetString; /** * Builder-function that appends a variable (`${VAR}`) to @@ -3568,7 +3568,7 @@ declare module 'vscode' { * be a range or a range and a placeholder text. The placeholder text should be the identifier of the symbol * which is being renamed - when omitted the text in the returned range is used. * - * *Note: * This function should throw an error or return a rejected thenable when the provided location + * *Note:* This function should throw an error or return a rejected thenable when the provided location * doesn't allow for a rename. * * @param document The document in which rename will be invoked. @@ -3622,7 +3622,7 @@ declare module 'vscode' { * @param tokenType The token type. * @param tokenModifiers The token modifiers. */ - push(range: Range, tokenType: string, tokenModifiers?: string[]): void; + push(range: Range, tokenType: string, tokenModifiers?: readonly string[]): void; /** * Finish and create a `SemanticTokens` instance. @@ -4893,7 +4893,7 @@ declare module 'vscode' { * @return Selection ranges or a thenable that resolves to such. The lack of a result can be * signaled by returning `undefined` or `null`. */ - provideSelectionRanges(document: TextDocument, positions: Position[], token: CancellationToken): ProviderResult; + provideSelectionRanges(document: TextDocument, positions: readonly Position[], token: CancellationToken): ProviderResult; } /** @@ -9522,7 +9522,7 @@ declare module 'vscode' { * @return A new Terminal. * @throws When running in an environment where a new process cannot be started. */ - export function createTerminal(name?: string, shellPath?: string, shellArgs?: string[] | string): Terminal; + export function createTerminal(name?: string, shellPath?: string, shellArgs?: readonly string[] | string): Terminal; /** * Creates a {@link Terminal} with a backing shell process. @@ -9830,7 +9830,7 @@ declare module 'vscode' { * * Note that mime types that cannot be sent to the extension will be omitted. */ - readonly dropMimeTypes: string[]; + readonly dropMimeTypes: readonly string[]; /** * The mime types that the {@link TreeDragAndDropController.handleDrag `handleDrag`} method of this `TreeDragAndDropController` may add to the tree data transfer. @@ -9838,7 +9838,7 @@ declare module 'vscode' { * * The recommended mime type of the tree (`application/vnd.code.tree.`) will be automatically added. */ - readonly dragMimeTypes: string[]; + readonly dragMimeTypes: readonly string[]; /** * When the user starts dragging items from this `DragAndDropController`, `handleDrag` will be called. @@ -9856,7 +9856,7 @@ declare module 'vscode' { * @param dataTransfer The data transfer associated with this drag. * @param token A cancellation token indicating that drag has been cancelled. */ - handleDrag?(source: T[], dataTransfer: DataTransfer, token: CancellationToken): Thenable | void; + handleDrag?(source: readonly T[], dataTransfer: DataTransfer, token: CancellationToken): Thenable | void; /** * Called when a drag and drop action results in a drop on the tree that this `DragAndDropController` belongs to. @@ -11000,7 +11000,7 @@ declare module 'vscode' { * * @param thenable A thenable that resolves to {@link TextEdit pre-save-edits}. */ - waitUntil(thenable: Thenable): void; + waitUntil(thenable: Thenable): void; /** * Allows to pause the event loop until the provided thenable resolved. @@ -11406,11 +11406,18 @@ declare module 'vscode' { * By default, all opened {@link workspace.workspaceFolders workspace folders} will be watched * for file changes recursively. * - * Additional folders can be added for file watching by providing a {@link RelativePattern} with - * a `base` that is outside of any of the currently opened workspace folders. If the `pattern` is - * complex (e.g. contains `**` or path segments), the folder will be watched recursively and - * otherwise will be watched non-recursively (i.e. only changes to the first level of the path - * will be reported). + * Additional paths can be added for file watching by providing a {@link RelativePattern} with + * a `base` path to watch. If the `pattern` is complex (e.g. contains `**` or path segments), + * the path will be watched recursively and otherwise will be watched non-recursively (i.e. only + * changes to the first level of the path will be reported). + * + * *Note* that requests for recursive file watchers for a `base` path that is inside the opened + * workspace are ignored given all opened {@link workspace.workspaceFolders workspace folders} are + * watched for file changes recursively by default. Non-recursive file watchers however are always + * supported, even inside the opened workspace because they allow to bypass the configured settings + * for excludes (`files.watcherExclude`). If you need to watch in a location that is typically + * excluded (for example `node_modules` or `.git` folder), then you can use a non-recursive watcher + * in the workspace for this purpose. * * If possible, keep the use of recursive watchers to a minimum because recursive file watching * is quite resource intense. @@ -13114,7 +13121,7 @@ declare module 'vscode' { * this execution. * @return A thenable that resolves when the operation finished. */ - replaceOutput(out: NotebookCellOutput | NotebookCellOutput[], cell?: NotebookCell): Thenable; + replaceOutput(out: NotebookCellOutput | readonly NotebookCellOutput[], cell?: NotebookCell): Thenable; /** * Append to the output of the cell that is executing or to another cell that is affected by this execution. @@ -13124,7 +13131,7 @@ declare module 'vscode' { * this execution. * @return A thenable that resolves when the operation finished. */ - appendOutput(out: NotebookCellOutput | NotebookCellOutput[], cell?: NotebookCell): Thenable; + appendOutput(out: NotebookCellOutput | readonly NotebookCellOutput[], cell?: NotebookCell): Thenable; /** * Replace all output items of existing cell output. @@ -13133,7 +13140,7 @@ declare module 'vscode' { * @param output Output object that already exists. * @return A thenable that resolves when the operation finished. */ - replaceOutputItems(items: NotebookCellOutputItem | NotebookCellOutputItem[], output: NotebookCellOutput): Thenable; + replaceOutputItems(items: NotebookCellOutputItem | readonly NotebookCellOutputItem[], output: NotebookCellOutput): Thenable; /** * Append output items to existing cell output. @@ -13142,7 +13149,7 @@ declare module 'vscode' { * @param output Output object that already exists. * @return A thenable that resolves when the operation finished. */ - appendOutputItems(items: NotebookCellOutputItem | NotebookCellOutputItem[], output: NotebookCellOutput): Thenable; + appendOutputItems(items: NotebookCellOutputItem | readonly NotebookCellOutputItem[], output: NotebookCellOutput): Thenable; } /** diff --git a/tabs-api-sample/vscode.proposed.tabs.d.ts b/tabs-api-sample/vscode.proposed.tabs.d.ts index 91ba6ade..273441b6 100644 --- a/tabs-api-sample/vscode.proposed.tabs.d.ts +++ b/tabs-api-sample/vscode.proposed.tabs.d.ts @@ -7,23 +7,54 @@ declare module 'vscode' { // https://github.com/Microsoft/vscode/issues/15178 + // TODO@API name alternatives for TabKind: TabInput, TabOptions, + + + /** + * The tab represents a single text based resource + */ export class TabKindText { + /** + * The uri represented by the tab. + */ readonly uri: Uri; constructor(uri: Uri); } + /** + * The tab represents two text based resources + * being rendered as a diff. + */ export class TabKindTextDiff { + /** + * The uri of the original text resource. + */ readonly original: Uri; + /** + * The uri of the modified text resource. + */ readonly modified: Uri; constructor(original: Uri, modified: Uri); } + /** + * The tab represents a custom editor. + */ export class TabKindCustom { + /** + * The uri which the tab is representing. + */ readonly uri: Uri; + /** + * The type of custom editor. + */ readonly viewType: string; constructor(uri: Uri, viewType: string); } + /** + * The tab represents a webview. + */ export class TabKindWebview { /** * The type of webview. Maps to {@linkcode WebviewPanel.viewType WebviewPanel's viewType} @@ -32,25 +63,48 @@ declare module 'vscode' { constructor(viewType: string); } + /** + * The tab represents a notebook. + */ export class TabKindNotebook { + /** + * The uri which the tab is representing. + */ readonly uri: Uri; + /** + * The type of notebook. Maps to {@linkcode NotebookDocument.notebookType NotebookDocuments's notebookType} + */ readonly notebookType: string; constructor(uri: Uri, notebookType: string); } + /** + * The tabs represents two notebooks in a diff configuration. + */ export class TabKindNotebookDiff { + /** + * The uri of the original notebook. + */ readonly original: Uri; + /** + * The uri of the modified notebook. + */ readonly modified: Uri; readonly notebookType: string; constructor(original: Uri, modified: Uri, notebookType: string); } + /** + * The tab represents a terminal in the editor area. + */ export class TabKindTerminal { constructor(); } /** - * Represents a tab within the window + * Represents a tab within a {@link TabGroup group of tabs}. + * Tabs are merely the graphical representation within the editor area. + * A backing editor is not a guarantee. */ export interface Tab { @@ -99,9 +153,25 @@ declare module 'vscode' { export const tabGroups: TabGroups; } + export interface TabChangeEvent { + // TODO@API consider: opened + readonly added: readonly Tab[]; + // TODO@API consider: closed (aligns with TabGroups.close(...)) + readonly removed: readonly Tab[]; + readonly changed: readonly Tab[]; + } + + /** + * Represents a group of tabs. A tab group itself consists of multiple tab + */ export interface TabGroup { /** - * Whether or not the group is currently active + * Whether or not the group is currently active. + * + * *Note* that only one tab group is active at a time, but that multiple tab + * groups can have an {@link TabGroup.aciveTab active tab}. + * + * @see {@link Tab.isActive} */ readonly isActive: boolean; @@ -111,8 +181,10 @@ declare module 'vscode' { readonly viewColumn: ViewColumn; /** - * The active tab in the group (this is the tab currently being rendered). - * There can be one active tab per group. There can only be one active group. + * The active {@link Tab tab} in the group. This is the tab which contents are currently + * being rendered. + * + * *Note* that there can be one active tab per group but there can only be one {@link TabGroups.activeTabGroup active group}. */ readonly activeTab: Tab | undefined; @@ -127,49 +199,43 @@ declare module 'vscode' { /** * All the groups within the group container */ - readonly groups: readonly TabGroup[]; + readonly all: readonly TabGroup[]; /** * The currently active group */ - // TOD@API name: maybe `activeGroup` to align with `groups` (which isn't tabGroups) readonly activeTabGroup: TabGroup; /** - * An {@link Event event} which fires when {@link TabGroup tab groups} has changed. + * An {@link Event event} which fires when {@link TabGroup tab groups} have changed. */ - readonly onDidChangeTabGroups: Event; + // TODO@API consider `TabGroupChangeEvent` similar to `TabChangeEvent` + readonly onDidChangeTabGroups: Event; /** - * An {@link Event event} which fires when a {@link Tab tabs} have changed. + * An {@link Event event} which fires when {@link Tab tabs} have changed. */ - readonly onDidChangeTabs: Event; - - /** - * An {@link Event} which fires when an active tab changes. - * Similar to {@link TabGroup.onDidChangeTabs} but only on tabs - * with isActive equal to true. - */ - readonly onDidChangeActiveTab: Event; - - /** - * An {@link Event} which fires when the active group changes. - * This does not fire when the properties within the group change. - */ - readonly onDidChangeActiveTabGroup: Event; + readonly onDidChangeTabs: Event; /** * Closes the tab. This makes the tab object invalid and the tab * should no longer be used for further actions. * Note: In the case of a dirty tab, a confirmation dialog will be shown which may be cancelled. If cancelled the tab is still valid - * @param tab The tab to close, must be reference equal to a tab given by the API + * + * @param tab The tab to close. * @param preserveFocus When `true` focus will remain in its current position. If `false` it will jump to the next tab. - * @returns A promise that resolves true when then tab is closed. Otherwise it will return false. - * If false is returned the tab is still valid. + * @returns A promise that resolves to `true` when all tabs have been closed */ - close(tab: Tab | Tab[], preserveFocus?: boolean): Thenable; - // TODO@API support to close "all" - // close(tab: TabGroup | TabGroup[], preserveFocus?: boolean): Thenable; + close(tab: Tab | readonly Tab[], preserveFocus?: boolean): Thenable; + + /** + * Closes the tab group. This makes the tab group object invalid and the tab group + * should no longer be used for furhter actions. + * @param tabGroup The tab group to close. + * @param preserveFocus When `true` focus will remain in its current position. + * @returns A promise that resolves to `true` when all tab groups have been closed + */ + close(tabGroup: TabGroup | readonly TabGroup[], preserveFocus?: boolean): Thenable; /** * Moves a tab to the given index within the column. @@ -180,9 +246,7 @@ declare module 'vscode' { * @param viewColumn The column to move the tab into * @param index The index to move the tab to */ - // TODO@API support TabGroup in addition to ViewColumn - // TODO@API support just index for moving inside current group - // TODO@API move a tag group + // TODO@API remove for now move(tab: Tab, viewColumn: ViewColumn, index: number, preserveFocus?: boolean): Thenable; } }