From 10ec38ada391b89f4a1836c060409ffa34a5e657 Mon Sep 17 00:00:00 2001 From: Tyler Nullmeier Date: Tue, 28 May 2024 13:18:15 -0500 Subject: [PATCH] Remove toc-trees-provider TocTreesProvider and TocTreeItem were only used in tests Move toggleTocTreesFilteringHandler to book-tocs --- client/specs/book-tocs.spec.ts | 77 ++++++++++++++++++++- client/src/book-tocs.ts | 58 +++++++++++++++- client/src/extension.ts | 2 +- client/src/toc-trees-provider.ts | 115 ------------------------------- 4 files changed, 133 insertions(+), 119 deletions(-) delete mode 100644 client/src/toc-trees-provider.ts diff --git a/client/specs/book-tocs.spec.ts b/client/specs/book-tocs.spec.ts index c9fa8be6..2cda5325 100644 --- a/client/specs/book-tocs.spec.ts +++ b/client/specs/book-tocs.spec.ts @@ -1,7 +1,9 @@ +import SinonRoot from 'sinon' +import type vscode from 'vscode' import { expect } from '@jest/globals' import { BookRootNode, type BookToc, type ClientTocNode, TocNodeKind } from '../../common/src/toc' -import { TocsTreeProvider } from '../src/book-tocs' +import { type BookOrTocNode, TocsTreeProvider, toggleTocTreesFilteringHandler } from '../src/book-tocs' const testTocPage: ClientTocNode = { type: TocNodeKind.Page, @@ -69,3 +71,76 @@ describe('Toc Provider', () => { expect(p.getParentBook(testToc)).toBe(undefined) }) }) + +describe('filtering', () => { + const sinon = SinonRoot.createSandbox() + afterEach(() => { sinon.restore() }) + it('toggleTocTreesFilteringHandler', async () => { + const revealStub = sinon.stub() + const toggleFilterStub = sinon.stub() + const getChildrenStub = sinon.stub() + const refreshStub = sinon.stub() + + const view: vscode.TreeView = { + reveal: revealStub + } as unknown as vscode.TreeView + const provider: TocsTreeProvider = { + toggleFilterMode: toggleFilterStub, + getChildren: getChildrenStub, + refresh: refreshStub, + getParent: () => undefined + } as unknown as TocsTreeProvider + const fakeChildren = [ + { type: BookRootNode.Singleton, tocTree: [{ type: TocNodeKind.Subbook, label: 'unit1', children: [{ type: TocNodeKind.Subbook, label: 'subcol1', children: [{ type: TocNodeKind.Page, label: 'm2', children: [] }] }] }] }, + { type: BookRootNode.Singleton, tocTree: [{ label: 'm1', children: [] }] } + ] + getChildrenStub.returns(fakeChildren) + + const handler = toggleTocTreesFilteringHandler(view, provider) + await handler() + expect(toggleFilterStub.callCount).toBe(1) + expect(getChildrenStub.callCount).toBe(1) + expect(revealStub.callCount).toBe(2) + expect(revealStub.getCalls().map(c => c.args)).toMatchSnapshot() + expect(refreshStub.callCount).toBe(0) + }) + it('toggleTocTreesFilteringHandler disables itself while revealing', async () => { + const revealStub = sinon.stub() + const toggleFilterStub = sinon.stub() + const getChildrenStub = sinon.stub() + const fakeChildren = [ + { label: 'col1', children: [{ label: 'm1', children: [] }] } + ] + getChildrenStub.returns(fakeChildren) + + const view: vscode.TreeView = { + reveal: revealStub + } as unknown as vscode.TreeView + const provider: TocsTreeProvider = { + toggleFilterMode: toggleFilterStub, + getChildren: getChildrenStub, + getParent: () => undefined + } as unknown as TocsTreeProvider + + const handler = toggleTocTreesFilteringHandler(view, provider) + // Invoke the handler the first time reveal is called to simulate a parallel + // user request without resorting to synthetic delay injection + revealStub.onCall(0).callsFake(handler) + await handler() + expect(toggleFilterStub.callCount).toBe(1) + expect(revealStub.callCount).toBe(1) + expect(getChildrenStub.callCount).toBe(1) + }) + it('toggleTocTreesFilteringHandler does not lock itself on errors', async () => { + const toggleFilterStub = sinon.stub().throws() + const view: vscode.TreeView = {} as unknown as vscode.TreeView + const provider: TocsTreeProvider = { + toggleFilterMode: toggleFilterStub + } as unknown as TocsTreeProvider + + const handler = toggleTocTreesFilteringHandler(view, provider) + try { await handler() } catch { } + try { await handler() } catch { } + expect(toggleFilterStub.callCount).toBe(2) + }) +}) diff --git a/client/src/book-tocs.ts b/client/src/book-tocs.ts index f907eef5..db1e49d3 100644 --- a/client/src/book-tocs.ts +++ b/client/src/book-tocs.ts @@ -1,12 +1,18 @@ -import { EventEmitter, type TreeItem, TreeItemCollapsibleState, Uri, type TreeDataProvider } from 'vscode' +import { EventEmitter, type TreeItem, TreeItemCollapsibleState, Uri, type TreeDataProvider, ThemeIcon } from 'vscode' import { type BookToc, type ClientTocNode, BookRootNode, TocNodeKind, type ClientPageish } from '../../common/src/toc' -import { TocItemIcon } from './toc-trees-provider' +import type vscode from 'vscode' export type BookOrTocNode = BookToc | ClientTocNode const toClientTocNode = (n: ClientPageish): ClientTocNode => ({ type: TocNodeKind.Page, value: n }) +export const TocItemIcon = { + Page: ThemeIcon.File, + Book: new ThemeIcon('book'), + Subbook: ThemeIcon.Folder +} + export class TocsTreeProvider implements TreeDataProvider { private readonly _onDidChangeTreeData = new EventEmitter() readonly onDidChangeTreeData = this._onDidChangeTreeData.event @@ -122,3 +128,51 @@ export class TocsTreeProvider implements TreeDataProvider { : this.getBookIndex(parentBook) } } + +export function toggleTocTreesFilteringHandler(view: vscode.TreeView, provider: TocsTreeProvider): () => Promise { + let revealing: boolean = false + + // We call the view.reveal API for all nodes with children to ensure the tree + // is fully expanded. This approach is used since attempting to simply call + // reveal on root notes with the max expand value of 3 doesn't seem to always + // fully expose leaf nodes for large trees. + function leafFinder(acc: ClientTocNode[], elements: BookOrTocNode[]) { + for (const el of elements) { + if (el.type === BookRootNode.Singleton) { + leafFinder(acc, el.tocTree) + } else if (el.type === TocNodeKind.Subbook) { + leafFinder(acc, el.children) + } else { + acc.push(el) + } + } + } + + return async () => { + // Avoid parallel processing of requests by ignoring if we're actively + // revealing + if (revealing) { return } + revealing = true + + try { + // Toggle data provider filter mode and reveal all children so the + // tree expands if it hasn't already + provider.toggleFilterMode() + const leaves: ClientTocNode[] = [] + leafFinder(leaves, provider.getChildren()) + const nodes3Up = new Set() // VSCode allows expanding up to 3 levels down + leaves.forEach(l => { + const p1 = provider.getParent(l) + const p2 = p1 === undefined ? undefined : /* istanbul ignore next */ provider.getParent(p1) + const p3 = p2 === undefined ? undefined : /* istanbul ignore next */ provider.getParent(p2) + /* istanbul ignore next */ + nodes3Up.add(p3 ?? p2 ?? p1 ?? l) + }) + for (const node of Array.from(nodes3Up).reverse()) { + await view.reveal(node, { expand: 3 }) + } + } finally { + revealing = false + } + } +} diff --git a/client/src/extension.ts b/client/src/extension.ts index dd81b9b2..26a80324 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -8,7 +8,7 @@ import { expect, ensureCatch, ensureCatchPromise, launchLanguageServer, populate import { OpenstaxCommand } from './extension-types' import { type ExtensionHostContext, type Panel, PanelManager } from './panel' import { ImageManagerPanel } from './panel-image-manager' -import { toggleTocTreesFilteringHandler } from './toc-trees-provider' +import { toggleTocTreesFilteringHandler } from './book-tocs' import { type BookOrTocNode, TocsTreeProvider } from './book-tocs' import { type BooksAndOrphans, EMPTY_BOOKS_AND_ORPHANS, ExtensionServerNotification } from '../../common/src/requests' import { readmeGenerator } from './generate-readme' diff --git a/client/src/toc-trees-provider.ts b/client/src/toc-trees-provider.ts deleted file mode 100644 index 4adb0d70..00000000 --- a/client/src/toc-trees-provider.ts +++ /dev/null @@ -1,115 +0,0 @@ -import vscode, { ThemeIcon } from 'vscode' -import { BookRootNode, type ClientTocNode, TocNodeKind } from '../../common/src/toc' -import { type TocsTreeProvider, type BookOrTocNode } from './book-tocs' -import { type ExtensionHostContext } from './panel' - -export const TocItemIcon = { - Page: ThemeIcon.File, - Book: new ThemeIcon('book'), - Subbook: ThemeIcon.Folder -} - -export class TocTreesProvider implements vscode.TreeDataProvider { - private readonly _onDidChangeTreeData: vscode.EventEmitter = new vscode.EventEmitter() - readonly onDidChangeTreeData: vscode.Event = this._onDidChangeTreeData.event - private isFilterMode = false - - constructor(private readonly context: ExtensionHostContext) { - this.context.events.onDidChangeWatchedFiles(this.refresh.bind(this)) - } - - toggleFilterMode(): void { - this.isFilterMode = !this.isFilterMode - this.refresh() - } - - refresh(): void { - this._onDidChangeTreeData.fire(undefined) - } - - getTreeItem(element: TocTreeItem): TocTreeItem { - if (this.isFilterMode && (element.description != null)) { - return new TocTreeItem( - element.iconPath, - `${element.label} (${element.description})`, - element.collapsibleState, - element.children, - element.command, - undefined - ) - } - return element - } - - getChildren(element?: TocTreeItem): TocTreeItem[] { - return element?.children ?? [] - } - - getParent(element: TocTreeItem): vscode.ProviderResult { - return element.parent - } -} - -export class TocTreeItem extends vscode.TreeItem { - parent: TocTreeItem | undefined = undefined - - constructor( - public readonly iconPath: ThemeIcon, - public readonly label: string, - public readonly collapsibleState: vscode.TreeItemCollapsibleState, - public readonly children: TocTreeItem[], - public readonly command?: vscode.Command, - public readonly description?: string - ) { - super(label, collapsibleState) - this.children.forEach(child => { child.parent = this }) - } -} - -export function toggleTocTreesFilteringHandler(view: vscode.TreeView, provider: TocsTreeProvider): () => Promise { - let revealing: boolean = false - - // We call the view.reveal API for all nodes with children to ensure the tree - // is fully expanded. This approach is used since attempting to simply call - // reveal on root notes with the max expand value of 3 doesn't seem to always - // fully expose leaf nodes for large trees. - function leafFinder(acc: ClientTocNode[], elements: BookOrTocNode[]) { - for (const el of elements) { - if (el.type === BookRootNode.Singleton) { - leafFinder(acc, el.tocTree) - } else if (el.type === TocNodeKind.Subbook) { - leafFinder(acc, el.children) - } else { - acc.push(el) - } - } - } - - return async () => { - // Avoid parallel processing of requests by ignoring if we're actively - // revealing - if (revealing) { return } - revealing = true - - try { - // Toggle data provider filter mode and reveal all children so the - // tree expands if it hasn't already - provider.toggleFilterMode() - const leaves: ClientTocNode[] = [] - leafFinder(leaves, provider.getChildren()) - const nodes3Up = new Set() // VSCode allows expanding up to 3 levels down - leaves.forEach(l => { - const p1 = provider.getParent(l) - const p2 = p1 === undefined ? undefined : /* istanbul ignore next */ provider.getParent(p1) - const p3 = p2 === undefined ? undefined : /* istanbul ignore next */ provider.getParent(p2) - /* istanbul ignore next */ - nodes3Up.add(p3 ?? p2 ?? p1 ?? l) - }) - for (const node of Array.from(nodes3Up).reverse()) { - await view.reveal(node, { expand: 3 }) - } - } finally { - revealing = false - } - } -}