diff --git a/client/package-lock.json b/client/package-lock.json index 637f1a00..de769f21 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -16,7 +16,7 @@ "@types/node": "^17.0.16", "@types/react-sortable-tree": "^0.3.15", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.64.0", + "@types/vscode": "^1.89.0", "@types/xmldom": "^0.1.31", "json-stable-stringify": "^1.0.1", "preact": "^10.6.5", @@ -148,9 +148,9 @@ "dev": true }, "node_modules/@types/vscode": { - "version": "1.64.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.64.0.tgz", - "integrity": "sha512-bSlAWz5WtcSL3cO9tAT/KpEH9rv5OBnm93OIIFwdCshaAiqr2bp1AUyEwW9MWeCvZBHEXc3V0fTYVdVyzDNwHA==", + "version": "1.89.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.89.0.tgz", + "integrity": "sha512-TMfGKLSVxfGfoO8JfIE/neZqv7QLwS4nwPwL/NwMvxtAY2230H2I4Z5xx6836pmJvMAzqooRQ4pmLm7RUicP3A==", "dev": true }, "node_modules/@types/xmldom": { @@ -708,9 +708,9 @@ "dev": true }, "@types/vscode": { - "version": "1.64.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.64.0.tgz", - "integrity": "sha512-bSlAWz5WtcSL3cO9tAT/KpEH9rv5OBnm93OIIFwdCshaAiqr2bp1AUyEwW9MWeCvZBHEXc3V0fTYVdVyzDNwHA==", + "version": "1.89.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.89.0.tgz", + "integrity": "sha512-TMfGKLSVxfGfoO8JfIE/neZqv7QLwS4nwPwL/NwMvxtAY2230H2I4Z5xx6836pmJvMAzqooRQ4pmLm7RUicP3A==", "dev": true }, "@types/xmldom": { diff --git a/client/package.json b/client/package.json index a56d8fba..1fb1fec7 100644 --- a/client/package.json +++ b/client/package.json @@ -11,7 +11,7 @@ "@types/node": "^17.0.16", "@types/react-sortable-tree": "^0.3.15", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.64.0", + "@types/vscode": "^1.89.0", "@types/xmldom": "^0.1.31", "json-stable-stringify": "^1.0.1", "preact": "^10.6.5", diff --git a/client/specs/__mocks__/vscode.js b/client/specs/__mocks__/vscode.js index 5a76ad17..5c4eedf1 100644 --- a/client/specs/__mocks__/vscode.js +++ b/client/specs/__mocks__/vscode.js @@ -136,6 +136,10 @@ class CodeLens {} class DocumentLink {} class CodeAction {} class CallHierarchyItem {} +class DataTransferItem { + constructor(value) { this.value = value } +} +class DataTransfer {} class ThemeIcon { static File = 'File' @@ -222,6 +226,8 @@ const vscode = { ProgressLocation, FileSystemError, TextEditorRevealType, + DataTransferItem, + DataTransfer }; module.exports = { ...vscode, default: vscode }; \ No newline at end of file diff --git a/client/specs/__snapshots__/book-tocs.spec.ts.snap b/client/specs/__snapshots__/book-tocs.spec.ts.snap index d58ff72e..228940fa 100644 --- a/client/specs/__snapshots__/book-tocs.spec.ts.snap +++ b/client/specs/__snapshots__/book-tocs.spec.ts.snap @@ -14,6 +14,7 @@ exports[`Toc Provider filters fileids when filtering is set 1`] = ` "command": "vscode.open", "title": "open", }, + "contextValue": "rename,delete", "description": "fileId", "iconPath": "File", "label": "title", @@ -39,6 +40,7 @@ exports[`Toc Provider filters fileids when filtering is set 2`] = ` "command": "vscode.open", "title": "open", }, + "contextValue": "rename,delete", "iconPath": "File", "label": "title (fileId)", "resourceUri": { @@ -63,6 +65,7 @@ exports[`Toc Provider filters fileids when filtering is set 3`] = ` "command": "vscode.open", "title": "open", }, + "contextValue": "rename,delete", "description": "fileId", "iconPath": "File", "label": "title", @@ -102,6 +105,7 @@ exports[`Toc Provider returns tree items for children 1`] = ` exports[`Toc Provider returns tree items for children 2`] = ` { "collapsibleState": "Collapsed", + "contextValue": "rename", "iconPath": "Folder", "label": "title", } @@ -121,6 +125,7 @@ exports[`Toc Provider returns tree items for children 3`] = ` "command": "vscode.open", "title": "open", }, + "contextValue": "rename", "description": "fileId", "iconPath": "File", "label": "title", @@ -131,3 +136,12 @@ exports[`Toc Provider returns tree items for children 3`] = ` }, } `; + +exports[`Toc Provider returns tree items for children 4`] = ` +{ + "collapsibleState": "Collapsed", + "contextValue": "rename", + "iconPath": "Folder", + "label": "title", +} +`; diff --git a/client/specs/book-tocs.spec.ts b/client/specs/book-tocs.spec.ts index 9379008c..c9fa8be6 100644 --- a/client/specs/book-tocs.spec.ts +++ b/client/specs/book-tocs.spec.ts @@ -12,10 +12,19 @@ const testTocPage: ClientTocNode = { fileId: 'fileId' } } +const testTocAncillary: ClientTocNode = { + type: TocNodeKind.Ancillary, + value: { + absPath: '/path/to/ancillary', + token: 'token', + title: 'title', + fileId: 'fileId' + } +} const testTocSubbook: ClientTocNode = { type: TocNodeKind.Subbook, value: { token: 'token', title: 'title' }, - children: [testTocPage] + children: [testTocPage, testTocAncillary] } const testToc: BookToc = { type: BookRootNode.Singleton, @@ -27,15 +36,17 @@ const testToc: BookToc = { licenseUrl: 'licenseUrl', tocTree: [testTocSubbook] } - describe('Toc Provider', () => { const p = new TocsTreeProvider() it('returns tree items for children', () => { expect(p.getTreeItem(testToc)).toMatchSnapshot() expect(p.getTreeItem(testTocSubbook)).toMatchSnapshot() expect(p.getTreeItem(testTocPage)).toMatchSnapshot() + expect(p.getTreeItem(testTocAncillary)).toMatchSnapshot() }) it('filters fileids when filtering is set', () => { + const p = new TocsTreeProvider() + p.update([testToc], []) expect(p.getTreeItem(testTocPage)).toMatchSnapshot() p.toggleFilterMode() expect(p.getTreeItem(testTocPage)).toMatchSnapshot() @@ -50,10 +61,11 @@ describe('Toc Provider', () => { expect(p.getTreeItem(nonloadedPage).label).toBe(`Loading... (${nonloadedPage.value.fileId})`) }) it('gets children and parents', () => { - p.update([testToc]) + p.update([testToc], []) expect(p.getChildren()).toEqual([testToc]) expect(p.getParent(testToc)).toBe(undefined) expect(p.getChildren(testToc)).toEqual(testToc.tocTree) expect(p.getParent(testToc.tocTree[0])).toBe(testToc) + expect(p.getParentBook(testToc)).toBe(undefined) }) }) diff --git a/client/specs/panel-toc-editor.spec.ts b/client/specs/panel-toc-editor.spec.ts index 740a4b64..5ccd7613 100644 --- a/client/specs/panel-toc-editor.spec.ts +++ b/client/specs/panel-toc-editor.spec.ts @@ -251,10 +251,10 @@ describe('Toc Editor', () => { expect(getMessage().type).toBe(TocModificationKind.SubbookRename) sinon.stub(vscode.window, 'showInputBox').returns(Promise.resolve('new_title')) - await p.handleMessage({ type: TocNodeKind.Page, title: 'foo', bookIndex: 0 }) + await p.handleMessage({ type: TocNodeKind.Page, title: 'foo', bookIndex: 0, parentNodeToken: undefined }) expect(getMessage().title).toBe('new_title') - await p.handleMessage({ type: TocNodeKind.Subbook, title: 'foo', bookIndex: 0, slug: 'subbook_slug' }) + await p.handleMessage({ type: TocNodeKind.Subbook, title: 'foo', bookIndex: 0, slug: 'subbook_slug', parentNodeToken: undefined }) expect(getMessage().title).toBe('new_title') }) it('disposes', () => { diff --git a/client/specs/tocs-event-handler.spec.ts b/client/specs/tocs-event-handler.spec.ts new file mode 100644 index 00000000..71ddd761 --- /dev/null +++ b/client/specs/tocs-event-handler.spec.ts @@ -0,0 +1,442 @@ +import { jest, expect } from '@jest/globals' +import { type BookOrTocNode, TocsTreeProvider } from '../src/book-tocs' +import { type ClientTocNode, TocNodeKind, type BookToc, BookRootNode, TocModificationKind, type TocModification, type CreateSubbookEvent, type CreatePageEvent, type CreateAncillaryEvent } from '../../common/src/toc' +import { TocsEventHandler, XFER_ITEM_ID, validateTitle } from '../src/tocs-event-handler' +import { type LanguageClient } from 'vscode-languageclient/node' +import { type ExtensionHostContext } from '../src/panel' +import { ExtensionServerRequest } from '../../common/src/requests' +import { DataTransferItem, type DataTransfer } from 'vscode' + +import vscode from 'vscode' + +const createPage = (title: string, token: string, fileId: string, absPath: string): ClientTocNode => ({ + type: TocNodeKind.Page, + value: { title, token, fileId, absPath } +}) + +const createBookToC = () => { + const testTocPage: ClientTocNode = { + type: TocNodeKind.Page, + value: { + absPath: '/path/to/file', + token: 'page-token', + title: 'title', + fileId: 'fileId' + } + } + const testTocAncillary: ClientTocNode = { + type: TocNodeKind.Ancillary, + value: { + absPath: '/path/to/ancillary', + token: 'token', + title: 'title', + fileId: 'fileId' + } + } + const testTocSubbook: ClientTocNode = { + type: TocNodeKind.Subbook, + value: { token: 'subbook-token', title: 'title' }, + children: [testTocPage, testTocAncillary] + } + const testToc: BookToc = { + type: BookRootNode.Singleton, + absPath: '/some/path', + uuid: 'uuid', + title: 'title', + slug: 'slug', + language: 'language', + licenseUrl: 'licenseUrl', + tocTree: [testTocSubbook] + } + return testToc +} + +const testToc = Object.freeze(createBookToC()) +const testTocSubbook = testToc.tocTree[0] as (ClientTocNode & { type: TocNodeKind.Subbook }) +const testTocPage = testTocSubbook.children[0] +const testTocAncillary = testTocSubbook.children[1] +const orphanedPage = createPage( + 'orphan', 'Token', 'file2', '/path/to/file2' +) + +const stub = (obj: T, overrides: Record) => + new Proxy( + obj, + { + get: (target, p) => Reflect.get(overrides, p) ?? Reflect.get(target, p) + } + ) + +describe('TocsEventHandler', () => { + let mockClient: LanguageClient + let mockHostContext: ExtensionHostContext + let tocsTreeProvider: TocsTreeProvider + let tocsEventHandler: TocsEventHandler + let askTitleMock: jest.Mock + const tocsEventHandlerOverrides = { + workspaceUri: '' + } + let sendRequestMock: jest.Mock + beforeEach(() => { + sendRequestMock = jest.fn() + askTitleMock = jest.fn().mockReturnValue('new-title') + mockClient = { + sendRequest: sendRequestMock + } as unknown as LanguageClient + mockHostContext = { + client: mockClient + } as unknown as ExtensionHostContext + tocsTreeProvider = new TocsTreeProvider() + tocsEventHandler = stub( + new TocsEventHandler(tocsTreeProvider, mockHostContext), tocsEventHandlerOverrides + ) + tocsEventHandler.askTitle = askTitleMock + tocsTreeProvider.update([testToc], []) + }) + describe('remove node', () => { + it('sends the correct event', async () => { + expect(sendRequestMock).not.toHaveBeenCalled() + await tocsEventHandler.removeNode(testTocPage) + expect(sendRequestMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toBeCalledWith( + ExtensionServerRequest.TocModification, + { + workspaceUri: tocsEventHandlerOverrides.workspaceUri, + event: { + type: TocModificationKind.Remove, + nodeToken: testTocPage.value.token, + bookIndex: 0 + } + } + ) + }) + it('ignores orphaned nodes', async () => { + expect(sendRequestMock).not.toHaveBeenCalled() + await tocsEventHandler.removeNode(orphanedPage) + expect(sendRequestMock).not.toHaveBeenCalled() + }) + it('rejects invalid nodes', async () => { + expect(sendRequestMock).not.toHaveBeenCalled() + await expect(tocsEventHandler.removeNode({} as unknown as BookOrTocNode)).rejects.toThrow(/token/) + expect(sendRequestMock).not.toHaveBeenCalled() + }) + }) + describe('move node', () => { + const testCases: Array<{ + subtitle: string + node: BookOrTocNode + target: BookOrTocNode + event: TocModification + }> = [ + { + subtitle: 'drag page onto page in subbook', + node: orphanedPage, + target: testTocPage, + event: { + type: TocModificationKind.Move, + bookIndex: 0, + newChildIndex: 0, + newParentToken: testTocSubbook.value.token, + nodeToken: orphanedPage.value.token + } + }, + { + subtitle: 'drag page onto subbook', + node: testTocPage, + target: testTocSubbook, + event: { + type: TocModificationKind.Move, + bookIndex: 0, + newChildIndex: 0, + newParentToken: testTocSubbook.value.token, + nodeToken: testTocPage.value.token + } + }, + { + subtitle: 'drag page onto book', + node: testTocPage, + target: testToc, + event: { + type: TocModificationKind.Move, + bookIndex: 0, + newChildIndex: 0, + newParentToken: undefined, + nodeToken: testTocPage.value.token + } + }, + { + subtitle: 'drag orphaned page onto subbook', + node: orphanedPage, + target: testTocSubbook, + event: { + type: TocModificationKind.Move, + bookIndex: 0, + newChildIndex: 0, + newParentToken: testTocSubbook.value.token, + nodeToken: orphanedPage.value.token + } + } + ] + testCases.forEach(({ subtitle, node, target, event }) => { + it(`sends the correct event (${subtitle})`, async () => { + expect(sendRequestMock).not.toHaveBeenCalled() + await tocsEventHandler.moveNode(node, target) + expect(sendRequestMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toBeCalledWith( + ExtensionServerRequest.TocModification, + { + workspaceUri: tocsEventHandlerOverrides.workspaceUri, + event + } + ) + }) + }) + it('uses the index of the target', async () => { + const newPage = createPage('my-page', 'my-token', 'my-file', '/your/abspath') + const toc = createBookToC() + const subbook = toc.tocTree[0] as (ClientTocNode & { type: TocNodeKind.Subbook }) + subbook.children.push(newPage) + tocsTreeProvider.update([toc], []) + expect(sendRequestMock).not.toHaveBeenCalled() + await tocsEventHandler.moveNode(orphanedPage, newPage) + expect(sendRequestMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toBeCalledWith( + ExtensionServerRequest.TocModification, + { + workspaceUri: tocsEventHandlerOverrides.workspaceUri, + event: { + type: TocModificationKind.Move, + bookIndex: 0, + newChildIndex: 2, + newParentToken: subbook.value.token, + nodeToken: orphanedPage.value.token + } + } + ) + }) + it('throws when node token is undefined', async () => { + await expect(tocsEventHandler.moveNode({} as unknown as BookOrTocNode, {} as unknown as BookOrTocNode)).rejects.toThrow(/token.*dragged.*node/) + }) + it('does not try to move a subbook into itself', async () => { + // Since the dragged node is moved to the same level as the drop target + // and testTocPage is a child of testTocSubbook, this could create a + // situation where LS is asked to move a subbook into itself. This is + // undefined behavior and should never happen. tl;dr: 1/0 + await tocsEventHandler.moveNode(testTocSubbook, testTocPage) + expect(sendRequestMock).not.toHaveBeenCalled() + }) + }) + + describe('add node', () => { + const testCases: Array<{ + subtitle: string + node: BookOrTocNode + event: CreatePageEvent | CreateSubbookEvent | CreateAncillaryEvent + }> = [ + { + subtitle: 'page', + node: testTocPage, + event: { + type: TocNodeKind.Page, + title: 'new-title', + bookIndex: 0, + parentNodeToken: 'subbook-token' + } + }, + { + subtitle: 'subbook', + node: testTocSubbook, + event: { + type: TocNodeKind.Subbook, + title: 'new-title', + slug: 'new-slug', + bookIndex: 0, + parentNodeToken: 'subbook-token' + } + }, + { + subtitle: 'ancillary', + node: testTocAncillary, + event: { + type: TocNodeKind.Ancillary, + title: 'new-title', + bookIndex: 0, + parentNodeToken: 'subbook-token' + } + }, + { + subtitle: 'wrong-ancillary', + node: orphanedPage, + event: { + type: TocNodeKind.Page, + title: 'new-title', + bookIndex: 0, + parentNodeToken: undefined + } + } + ] + testCases.forEach(({ subtitle, node, event }) => { + it(`checks if the title: (${subtitle}) is valid`, async () => { + const validation = validateTitle(subtitle) + expect(validation).toBeUndefined() + const invalid = validateTitle('') + expect(invalid).toBe('Title cannot be empty') + }) + + it(`sends the correct event (${subtitle})`, async () => { + expect(sendRequestMock).toHaveBeenCalledTimes(0) + await tocsEventHandler.addNode(event.type, node, 'new-slug') + expect(askTitleMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toHaveBeenCalledWith( + ExtensionServerRequest.TocModification, + { + workspaceUri: tocsEventHandlerOverrides.workspaceUri, + event + } + ) + }) + }) + }) + + describe('rename node', () => { + const testCases: Array<{ + subtitle: string + node: BookOrTocNode + title: string + event: TocModification + }> = [ + { + subtitle: 'rename page', + node: testTocPage, + title: 'title', + event: { + type: TocModificationKind.PageRename, + nodeToken: testTocPage.value.token, + newTitle: 'new-title', + bookIndex: 0 + } + }, + { + subtitle: 'subbook', + node: testTocSubbook, + title: 'title', + event: { + type: TocModificationKind.SubbookRename, + nodeToken: testTocSubbook.value.token, + newTitle: 'new-title', + bookIndex: 0 + } + }, + { + subtitle: 'ancillary', + node: testTocAncillary, + title: 'title', + event: { + type: TocModificationKind.AncillaryRename, + nodeToken: testTocAncillary.value.token, + newTitle: 'new-title', + bookIndex: 0 + } + } + ] + testCases.forEach(({ subtitle, node, title, event }) => { + it(`sends the correct event (${subtitle})`, async () => { + expect(sendRequestMock).toHaveBeenCalledTimes(0) + await tocsEventHandler.renameNode(node) + expect(askTitleMock).toHaveBeenCalledTimes(1) + expect(askTitleMock).toHaveBeenCalledWith(title) + expect(sendRequestMock).toHaveBeenCalledTimes(1) + expect(sendRequestMock).toHaveBeenCalledWith( + ExtensionServerRequest.TocModification, + { + workspaceUri: tocsEventHandlerOverrides.workspaceUri, + event + } + ) + }) + }) + }) + + describe('askTitle', () => { + let withSubbedEvents: TocsEventHandler + beforeEach(() => { + withSubbedEvents = stub( + new TocsEventHandler(tocsTreeProvider, mockHostContext), + tocsEventHandlerOverrides + ) + }) + it('should prompt the user for a title and validate the input', async () => { + let result = await withSubbedEvents.askTitle('title') + expect(result).toBeUndefined() + expect(vscode.window.showInputBox).toBeCalledTimes(1) + + result = await withSubbedEvents.askTitle('default-title') + expect(result).toBeUndefined() + expect(vscode.window.showInputBox).toBeCalledTimes(2) + expect(vscode.window.showInputBox).toHaveBeenCalledWith({ + prompt: 'Please enter the title', + value: 'default-title', + validateInput: expect.any(Function) + }) + }) + }) + + describe('workspaceUri', () => { + it('errors when value is null', async () => { + const t = new TocsEventHandler(tocsTreeProvider, mockHostContext) + // Expected to fail in test setting + expect(() => Reflect.get(t, 'workspaceUri')).toThrow() + }) + }) + describe('handleDrag', () => { + it('sets the data transfer item', () => { + const dt = { set: jest.fn() } + tocsEventHandler.handleDrag([orphanedPage], dt as any) + expect(dt.set).toHaveBeenCalledWith( + XFER_ITEM_ID, + new DataTransferItem(orphanedPage) + ) + }) + }) + describe('handleDrop', () => { + let removeNodeStub: jest.Mock + let moveNodeStub: jest.Mock + let withSubbedEvents: TocsEventHandler + beforeEach(() => { + removeNodeStub = jest.fn() + moveNodeStub = jest.fn() + withSubbedEvents = stub( + new TocsEventHandler(tocsTreeProvider, mockHostContext), + { + ...tocsEventHandlerOverrides, + removeNode: removeNodeStub, + moveNode: moveNodeStub + }) + }) + it('throws if it drag target is invalid', async () => { + const dt = { get: () => undefined } as unknown as DataTransfer + await expect(withSubbedEvents.handleDrop(testTocPage, dt)).rejects.toThrow(/drag/) + expect(removeNodeStub).not.toHaveBeenCalled() + expect(moveNodeStub).not.toHaveBeenCalled() + }) + it('throws if the drop target is invalid', async () => { + const dt = { get: () => ({ value: orphanedPage }) } as unknown as DataTransfer + await expect(withSubbedEvents.handleDrop(undefined, dt)).rejects.toThrow(/drop/) + expect(removeNodeStub).not.toHaveBeenCalled() + expect(moveNodeStub).not.toHaveBeenCalled() + }) + // it('calls removeNode when the target is a book', async () => { + // const dt = { get: () => ({ value: orphanedPage }) } as unknown as DataTransfer + // await withSubbedEvents.handleDrop(testToc, dt) + // expect(removeNodeStub).toHaveBeenCalledWith(orphanedPage) + // expect(moveNodeStub).not.toHaveBeenCalled() + // }) + it('calls moveNode when the target is anything else', async () => { + const dt = { get: () => ({ value: orphanedPage }) } as unknown as DataTransfer + await withSubbedEvents.handleDrop(testTocPage, dt) + expect(removeNodeStub).not.toHaveBeenCalled() + expect(moveNodeStub).toHaveBeenCalledWith(orphanedPage, testTocPage) + }) + }) +}) diff --git a/client/src/book-tocs.ts b/client/src/book-tocs.ts index 9215a009..f907eef5 100644 --- a/client/src/book-tocs.ts +++ b/client/src/book-tocs.ts @@ -1,20 +1,24 @@ -import { EventEmitter, TreeItemCollapsibleState, Uri, type TreeDataProvider } from 'vscode' +import { EventEmitter, type TreeItem, TreeItemCollapsibleState, Uri, type TreeDataProvider } from 'vscode' -import { type BookToc, type ClientTocNode, BookRootNode, TocNodeKind } from '../../common/src/toc' +import { type BookToc, type ClientTocNode, BookRootNode, TocNodeKind, type ClientPageish } from '../../common/src/toc' import { TocItemIcon } from './toc-trees-provider' export type BookOrTocNode = BookToc | ClientTocNode +const toClientTocNode = (n: ClientPageish): ClientTocNode => ({ type: TocNodeKind.Page, value: n }) + export class TocsTreeProvider implements TreeDataProvider { private readonly _onDidChangeTreeData = new EventEmitter() readonly onDidChangeTreeData = this._onDidChangeTreeData.event public includeFileIdsForFilter = false private bookTocs: BookToc[] + private orphans: BookOrTocNode[] private readonly parentsMap = new Map() constructor() { this.bookTocs = [] + this.orphans = [] } public toggleFilterMode() { @@ -22,10 +26,11 @@ export class TocsTreeProvider implements TreeDataProvider { this._onDidChangeTreeData.fire() } - public update(n: BookToc[]) { + public update(n: BookToc[], o: ClientPageish[]) { this.bookTocs = n this.parentsMap.clear() this.bookTocs.forEach(n => { this.recAddParent(n) }) + this.orphans = o.map(toClientTocNode) this._onDidChangeTreeData.fire() } @@ -37,7 +42,13 @@ export class TocsTreeProvider implements TreeDataProvider { }) } - public getTreeItem(node: BookOrTocNode) { + public getTreeItem(node: BookOrTocNode): TreeItem { + const capabilities: string[] = [ + 'rename' + ] + if (this.getParent(node) !== undefined) { + capabilities.push('delete') + } if (node.type === BookRootNode.Singleton) { const uri = Uri.parse(node.absPath) return { @@ -57,13 +68,15 @@ export class TocsTreeProvider implements TreeDataProvider { iconPath: TocItemIcon.Page, collapsibleState: TreeItemCollapsibleState.None, resourceUri: uri, - command: { title: 'open', command: 'vscode.open', arguments: [uri] } + command: { title: 'open', command: 'vscode.open', arguments: [uri] }, + contextValue: capabilities.join(',') } } else { return { iconPath: TocItemIcon.Subbook, collapsibleState: TreeItemCollapsibleState.Collapsed, - label: node.value.title + label: node.value.title, + contextValue: capabilities.join(',') } } } @@ -71,10 +84,10 @@ export class TocsTreeProvider implements TreeDataProvider { public getChildren(node?: BookOrTocNode) { let kids: BookOrTocNode[] = [] if (node === undefined) { - return this.bookTocs + return [...this.bookTocs, ...this.orphans] } else if (node.type === BookRootNode.Singleton) { kids = node.tocTree - } else if (node.type === TocNodeKind.Page) { + } else if (node.type === TocNodeKind.Page || node.type === TocNodeKind.Ancillary) { kids = [] } else { kids = node.children @@ -85,4 +98,27 @@ export class TocsTreeProvider implements TreeDataProvider { public getParent(node: BookOrTocNode) { return this.parentsMap.get(node) } + + public getParentBook(node: BookOrTocNode): BookToc | undefined { + const recursiveFindParent = (n: BookOrTocNode | undefined): BookToc | undefined => { + if (n === undefined) return undefined + if (n.type === BookRootNode.Singleton) return n + return recursiveFindParent(this.getParent(n)) + } + // If the original node is a book, it has no parent + return node.type === BookRootNode.Singleton + ? undefined + : recursiveFindParent(node) + } + + public getBookIndex(node: BookToc) { + return this.bookTocs.findIndex((b) => b.absPath === node.absPath) + } + + public getParentBookIndex(node: BookOrTocNode) { + const parentBook = this.getParentBook(node) + return parentBook === undefined + ? undefined + : this.getBookIndex(parentBook) + } } diff --git a/client/src/extension.ts b/client/src/extension.ts index d950980d..24a84bb9 100644 --- a/client/src/extension.ts +++ b/client/src/extension.ts @@ -13,9 +13,12 @@ import { toggleTocTreesFilteringHandler } from './toc-trees-provider' import { type BookOrTocNode, TocsTreeProvider } from './book-tocs' import { type BooksAndOrphans, EMPTY_BOOKS_AND_ORPHANS, ExtensionServerNotification } from '../../common/src/requests' import { readmeGenerator } from './generate-readme' +import { TocsEventHandler } from './tocs-event-handler' +import { TocNodeKind } from '../../common/src/toc' let tocTreesView: vscode.TreeView let tocTreesProvider: TocsTreeProvider +let tocEventHandler: TocsEventHandler let client: LanguageClient const onDidChangeWatchedFilesEmitter: vscode.EventEmitter = new vscode.EventEmitter() const onDidChangeWatchedFiles = onDidChangeWatchedFilesEmitter.event @@ -91,9 +94,10 @@ function doRest(client: LanguageClient): ExtensionExports { const imageManagerPanelManager = new PanelManager(hostContext, ImageManagerPanel) tocTreesProvider = new TocsTreeProvider() + tocEventHandler = new TocsEventHandler(tocTreesProvider, hostContext) client.onNotification(ExtensionServerNotification.BookTocs, (params: BooksAndOrphans) => { hostContext.bookTocs = params // When a panel opens, make sure it has the latest bookTocs - tocTreesProvider.update(params.books) + tocTreesProvider.update(params.books, params.orphans) /* istanbul ignore next */ void tocPanelManager.panel()?.update(params) }) @@ -104,9 +108,14 @@ function doRest(client: LanguageClient): ExtensionExports { vscode.commands.registerCommand(OpenstaxCommand.SHOW_CNXML_PREVIEW, cnxmlPreviewPanelManager.revealOrNew.bind(cnxmlPreviewPanelManager)) vscode.commands.registerCommand('openstax.pushContent', ensureCatch(pushContent(hostContext))) vscode.commands.registerCommand('openstax.generateReadme', ensureCatch(readmeGenerator(hostContext))) - tocTreesView = vscode.window.createTreeView('tocTrees', { treeDataProvider: tocTreesProvider, showCollapseAll: true }) + tocTreesView = vscode.window.createTreeView('tocTrees', { treeDataProvider: tocTreesProvider, showCollapseAll: true, dragAndDropController: tocEventHandler }) vscode.commands.registerCommand('openstax.toggleTocTreesFiltering', ensureCatch(toggleTocTreesFilteringHandler(tocTreesView, tocTreesProvider))) + vscode.commands.registerCommand('openstax.addAncillaryToToc', ensureCatch(async (node: BookOrTocNode) => { await tocEventHandler.addNode(TocNodeKind.Ancillary, node, 'test') })) + vscode.commands.registerCommand('openstax.addPageToToc', ensureCatch(async (node: BookOrTocNode) => { await tocEventHandler.addNode(TocNodeKind.Page, node, 'test') })) + vscode.commands.registerCommand('openstax.addSubBookToToc', ensureCatch(async (node: BookOrTocNode) => { await tocEventHandler.addNode(TocNodeKind.Subbook, node, 'test') })) vscode.commands.registerCommand('openstax.validateContent', ensureCatch(validateContent)) + vscode.commands.registerCommand('openstax.removeNode', ensureCatch(async (node: BookOrTocNode) => { await tocEventHandler.removeNode(node) })) + vscode.commands.registerCommand('openstax.renameNode', ensureCatch(async (node: BookOrTocNode) => { await tocEventHandler.renameNode(node) })) void ensureCatchPromise(setDefaultGitConfig()) void ensureCatchPromise(initPrivateSubmodule(hostContext)) diff --git a/client/src/panel-toc-editor.ts b/client/src/panel-toc-editor.ts index 8ffa3534..698cd613 100644 --- a/client/src/panel-toc-editor.ts +++ b/client/src/panel-toc-editor.ts @@ -29,7 +29,7 @@ export type PanelIncomingMessage = ( ) export type TreeItemWithToken = TreeItemUI & ({ - type: TocNodeKind.Page + type: TocNodeKind.Page | TocNodeKind.Ancillary token: string title: string | undefined fileId: string @@ -51,7 +51,7 @@ export interface PanelState { } function toTreeItem(n: ClientTocNode): TreeItemWithToken { - if (n.type === TocNodeKind.Page) { + if (n.type === TocNodeKind.Page || n.type === TocNodeKind.Ancillary) { return { type: n.type, token: n.value.token, @@ -162,7 +162,7 @@ export class TocEditorPanel extends Panel() function recAddModules(n: ClientTocNode) { - if (n.type === TocNodeKind.Page) { + if (n.type === TocNodeKind.Page || n.type === TocNodeKind.Ancillary) { allModules.add(n.value) } else { n.children.forEach(recAddModules) diff --git a/client/src/tocs-event-handler.ts b/client/src/tocs-event-handler.ts new file mode 100644 index 00000000..dd609818 --- /dev/null +++ b/client/src/tocs-event-handler.ts @@ -0,0 +1,213 @@ +import vscode from 'vscode' +import { type TocsTreeProvider, type BookOrTocNode } from './book-tocs' +import { type TocModification, TocModificationKind, type TocModificationParams, TocNodeKind, BookRootNode, type CreatePageEvent, type CreateSubbookEvent, type CreateAncillaryEvent } from '../../common/src/toc' +import { ExtensionServerRequest } from '../../common/src/requests' +import { expect, getRootPathUri } from './utils' +import { type ExtensionHostContext } from './panel' + +const getNodeToken = (node: BookOrTocNode) => { + return node.type === TocNodeKind.Page || node.type === TocNodeKind.Subbook || node.type === TocNodeKind.Ancillary + ? node.value.token + : undefined +} + +export const validateTitle = (title: string) => { + return title.trim().length === 0 ? 'Title cannot be empty' : undefined +} + +export const XFER_ITEM_ID = 'application/vnd.code.tree.tocTrees' + +export class TocsEventHandler implements vscode.TreeDragAndDropController { + constructor( + private readonly tocTreesProvider: TocsTreeProvider, + private readonly context: ExtensionHostContext + ) {} + + dragMimeTypes: readonly string[] = ['application/xml'] + dropMimeTypes: readonly string[] = this.dragMimeTypes + + private get workspaceUri() { + return expect(getRootPathUri(), 'Could not get root path uri').toString() + } + + private async fireEvent(event: TocModification | CreateSubbookEvent | CreatePageEvent | CreateAncillaryEvent) { + const workspaceUri = this.workspaceUri + const params: TocModificationParams = { workspaceUri, event } + await this.context.client.sendRequest( + ExtensionServerRequest.TocModification, + params + ) + } + + async moveNode(node: BookOrTocNode, target: BookOrTocNode) { + const nodeToken = expect( + getNodeToken(node), + 'BUG: Could not get token of dragged node' + ) + let newParentToken: string | undefined + let newChildIndex = 0 + let bookIndex: number + if (target.type === BookRootNode.Singleton) { + // In case of book drop target, use book index directly + bookIndex = this.tocTreesProvider.getBookIndex(target) + } else { + bookIndex = expect( + this.tocTreesProvider.getParentBookIndex(target), + 'BUG: Could not get index of target\'s parent book' + ) + if (target.type === TocNodeKind.Subbook) { + newParentToken = getNodeToken(target) + } else { + const newParent = expect( + this.tocTreesProvider.getParent(target), + 'BUG: Could not get new parent node' + ) + const targetToken = expect( + getNodeToken(target), + 'BUG: Could not get target token' + ) + newParentToken = getNodeToken(newParent) + newChildIndex = this.tocTreesProvider + .getChildren(newParent) + .findIndex((node) => getNodeToken(node) === targetToken) + } + // Do not try to move a book/subbook into itself + if (newParentToken === nodeToken) { return } + } + const event: TocModification = { + type: TocModificationKind.Move, + nodeToken, + newParentToken, + newChildIndex, + bookIndex + } + await this.fireEvent(event) + } + + async removeNode(node: BookOrTocNode) { + const nodeToken = expect( + getNodeToken(node), + 'BUG: Could not get token of node' + ) + const bookIndex = this.tocTreesProvider.getParentBookIndex(node) + if (bookIndex === undefined) { return } + const event: TocModification = { + type: TocModificationKind.Remove, + nodeToken, + bookIndex + } + await this.fireEvent(event) + } + + async askTitle(title?: string): Promise { + return await vscode.window.showInputBox({ + prompt: 'Please enter the title', + /* istanbul ignore next */ + value: title, + validateInput: validateTitle + }) + } + + async addNode(nodeType: TocNodeKind, node: BookOrTocNode, slug: string | undefined) { + const title = await this.askTitle() + /* istanbul ignore next */ + if (title === undefined) { return } + let parentNodeToken: string | undefined + let bookIndex: number | undefined = 0 + if (node !== undefined) { + bookIndex = this.tocTreesProvider.getParentBookIndex(node) + if (bookIndex === undefined) { bookIndex = 0 } + if (node.type === TocNodeKind.Subbook) { + parentNodeToken = getNodeToken(node) + } else if (node.type === TocNodeKind.Page || node.type === TocNodeKind.Ancillary) { + const parent = this.tocTreesProvider.getParent(node) + if (parent !== undefined) { + parentNodeToken = getNodeToken(parent) + } + } + } + if (nodeType === TocNodeKind.Page) { + const event: CreatePageEvent = { + type: TocNodeKind.Page, + title, + bookIndex, + parentNodeToken + } + await this.fireEvent(event) + } else if (nodeType === TocNodeKind.Subbook) { + const event: CreateSubbookEvent = { + type: TocNodeKind.Subbook, + title, + slug, + bookIndex, + parentNodeToken + } + await this.fireEvent(event) + } else if (nodeType === TocNodeKind.Ancillary) { + const event: CreateAncillaryEvent = { + type: TocNodeKind.Ancillary, + title, + bookIndex, + parentNodeToken + } + await this.fireEvent(event) + } + } + + async renameNode(node: BookOrTocNode) { + // TODO Implement the rename functionality using inline editing (wait for the API to be available) + // https://github.com/microsoft/vscode/issues/97190 + // https://stackoverflow.com/questions/70594061/change-an-existing-label-name-in-tree-view-vscode-extension + let oldTitle: string | undefined = '' + if (node.type !== BookRootNode.Singleton && 'title' in node.value) { + /* istanbul ignore next */ + oldTitle = node.value.title + } + const newTitle = await this.askTitle(oldTitle) + const nodeToken = expect( + getNodeToken(node), + 'BUG: Could not get token of renamed node' + ) + /* istanbul ignore next */ + if (newTitle === undefined) { return } + const bookIndex = expect(this.tocTreesProvider.getParentBookIndex(node), 'BUG: Could not get index of parent book') + if (node.type === TocNodeKind.Subbook) { + const event: TocModification = { + type: TocModificationKind.SubbookRename, + newTitle, + nodeToken, + bookIndex + } + await this.fireEvent(event) + } else if (node.type === TocNodeKind.Page) { + const event: TocModification = { + type: TocModificationKind.PageRename, + newTitle, + nodeToken, + bookIndex + } + await this.fireEvent(event) + } else if (node.type === TocNodeKind.Ancillary) { + const event: TocModification = { + type: TocModificationKind.AncillaryRename, + newTitle, + nodeToken, + bookIndex + } + await this.fireEvent(event) + } + } + + handleDrag(source: readonly BookOrTocNode[], dataTransfer: vscode.DataTransfer): void { + dataTransfer.set(XFER_ITEM_ID, new vscode.DataTransferItem(source[0])) + } + + async handleDrop(target: BookOrTocNode | undefined, dataTransfer: vscode.DataTransfer): Promise { + const dragging: BookOrTocNode | undefined = dataTransfer.get(XFER_ITEM_ID)?.value + if (dragging?.type === undefined) throw new Error('BUG: Bad drag target') + if (target === undefined) throw new Error('BUG: Bad drop target') + if (target !== dragging && dragging.type !== BookRootNode.Singleton) { + await this.moveNode(dragging, target) + } + } +} diff --git a/client/static/xsd/cnxml.xsd b/client/static/xsd/cnxml.xsd index 8d159598..807ac33e 100644 --- a/client/static/xsd/cnxml.xsd +++ b/client/static/xsd/cnxml.xsd @@ -47,6 +47,9 @@ + + + diff --git a/common/src/toc.ts b/common/src/toc.ts index 71dd99cf..bd300a08 100644 --- a/common/src/toc.ts +++ b/common/src/toc.ts @@ -2,7 +2,8 @@ // This enum is also hardcoded in the toc-editor Webview export enum TocNodeKind { Subbook = 'TocNodeKind.Subbook', - Page = 'TocNodeKind.Page' + Page = 'TocNodeKind.Page', + Ancillary = 'TocNodeKind.Ancillary' } export enum BookRootNode { @@ -10,20 +11,24 @@ export enum BookRootNode { } export enum TocModificationKind { + Add = 'TocModificationKind.Add', Move = 'TocModificationKind.Move', Remove = 'TocModificationKind.Remove', PageRename = 'TocModificationKind.PageRename', SubbookRename = 'TocModificationKind.SubbookRename', + AncillaryRename = 'TocModificationKind.AncillaryRename' } -type TocNode = TocSubbook | TocPage +type TocNode = TocSubbook | TocPage | TocAncillary export interface TocSubbook { readonly type: TocNodeKind.Subbook, children: Array>, value: I } export interface TocPage { readonly type: TocNodeKind.Page, value: L } +export interface TocAncillary { readonly type: TocNodeKind.Ancillary, value: L } export type Token = string export interface ClientPageish { token: Token, title: string | undefined, fileId: string, absPath: string } +export interface ClientAncillaryish { token: Token, title: string | undefined, fileId: string, absPath: string } export interface ClientSubbookish { token: Token, title: string } -export type ClientTocNode = TocNode +export type ClientTocNode = TocNode export interface BookToc { readonly type: BookRootNode.Singleton @@ -38,9 +43,9 @@ export interface BookToc { export interface TocModificationParams { workspaceUri: string - event: TocModification | CreateSubbookEvent | CreatePageEvent + event: TocModification | CreateSubbookEvent | CreatePageEvent | CreateAncillaryEvent } -export type TocModification = (TocMoveEvent | TocRemoveEvent | PageRenameEvent | SubbookRenameEvent) +export type TocModification = (TocMoveEvent | TocRemoveEvent | PageRenameEvent | SubbookRenameEvent | AncillaryRenameEvent) export interface TocMoveEvent { readonly type: TocModificationKind.Move readonly nodeToken: Token @@ -67,14 +72,31 @@ export interface SubbookRenameEvent { readonly bookIndex: number } +export interface AncillaryRenameEvent { + readonly type: TocModificationKind.AncillaryRename + readonly newTitle: string + readonly nodeToken: Token + readonly bookIndex: number +} + export interface CreateSubbookEvent { readonly type: TocNodeKind.Subbook readonly title: string - readonly slug: string + readonly slug: string | undefined readonly bookIndex: number + readonly parentNodeToken: Token | undefined } + +export interface CreateAncillaryEvent { + readonly type: TocNodeKind.Ancillary + readonly title: string + readonly bookIndex: number + readonly parentNodeToken: Token | undefined +} + export interface CreatePageEvent { readonly type: TocNodeKind.Page readonly title: string readonly bookIndex: number + readonly parentNodeToken: Token | undefined } diff --git a/package.json b/package.json index b6b1e633..d75fa3c1 100644 --- a/package.json +++ b/package.json @@ -67,16 +67,46 @@ "title": "Generate README", "category": "Openstax" }, + { + "command": "openstax.addAncillaryToToc", + "title": "Add Ancillary", + "category": "Openstax", + "icon": "$(person-add)" + }, + { + "command": "openstax.addPageToToc", + "title": "Add Page", + "category": "Openstax", + "icon": "$(file-add)" + }, + { + "command": "openstax.addSubBookToToc", + "title": "Add Sub Book", + "category": "Openstax", + "icon": "$(file-directory-create)" + }, { "command": "openstax.toggleTocTreesFiltering", "title": "Toggle ToC Filtering", "category": "Openstax", - "icon": "$(filter)" + "icon": "$(expand-all)" }, { "command": "openstax.validateContent", "title": "Validate Content", "category": "Openstax" + }, + { + "command": "openstax.renameNode", + "title": "Rename Node", + "category": "Openstax", + "icon": "$(pencil)" + }, + { + "command": "openstax.removeNode", + "title": "Remove Node", + "category": "Openstax", + "icon": "$(trash)" } ], "viewsContainers": { @@ -140,6 +170,33 @@ "command": "openstax.toggleTocTreesFiltering", "when": "view == tocTrees", "group": "navigation" + }, + { + "command": "openstax.addAncillaryToToc", + "when": "view == tocTrees", + "group": "navigation" + }, + { + "command": "openstax.addSubBookToToc", + "when": "view == tocTrees", + "group": "navigation" + }, + { + "command": "openstax.addPageToToc", + "when": "view == tocTrees", + "group": "navigation" + } + ], + "view/item/context": [ + { + "command": "openstax.renameNode", + "when": "view == tocTrees && viewItem =~ /,?rename,?/", + "group": "inline" + }, + { + "command": "openstax.removeNode", + "when": "view == tocTrees && viewItem =~ /,?delete,?/", + "group": "inline" } ] }, diff --git a/server/src/book-toc-utils.ts b/server/src/book-toc-utils.ts index a5b87301..2097a900 100644 --- a/server/src/book-toc-utils.ts +++ b/server/src/book-toc-utils.ts @@ -99,7 +99,7 @@ function recTree(tocIdMap: IdMap, parent } function recBuild(doc: Document, node: ClientTocNode): Element { - if (node.type === TocNodeKind.Page) { + if (node.type === TocNodeKind.Page || node.type === TocNodeKind.Ancillary) { const ret = doc.createElementNS(NS_COLLECTION, 'col:module') ret.setAttribute('document', node.value.fileId) return ret diff --git a/server/src/model-manager.spec.ts b/server/src/model-manager.spec.ts index 4f398f82..c1f44144 100644 --- a/server/src/model-manager.spec.ts +++ b/server/src/model-manager.spec.ts @@ -766,20 +766,70 @@ describe('modifyToc()', () => { expect(tocEntry.page).toBe(orphan) } }) + it('moves a Page between books', async () => { + mockfs({ + collections: { + 'book-a.collection.xml': bookMaker({ + slug: 'book-a', + toc: [{ title: 'subbook-a', children: ['m2468'] }] + }), + 'book-b.collection.xml': bookMaker({ + slug: 'book-b', + toc: [{ title: 'subbook-b', children: ['m1357'] }] + }) + }, + 'META-INF/books.xml': bundleMaker({ + books: ['book-a', 'book-b'] + }), + 'modules/m2468/index.cnxml': pageMaker({ uuid: 'somethig' }), + 'modules/m1357/index.cnxml': pageMaker({}) + }) + + const bundle = new Bundle(FS_PATH_HELPER, process.cwd()) + const manager = new ModelManager(bundle, conn, (p) => { params = p }) + await manager.loadEnoughForOrphans() + const { books } = params + const subbookA = books[0].tocTree[0] + let subbookB = books[1].tocTree[0] + expect(subbookA).toBeDefined() + expect(subbookB).toBeDefined() + if (subbookA.type !== TocNodeKind.Subbook) { throw new Error('Expected subbook') } + if (subbookB.type !== TocNodeKind.Subbook) { throw new Error('Expected subbook') } + const page1A = subbookA.children[0] + const page1B = subbookB.children[0] + + const evt1: TocModification = { + type: TocModificationKind.Move, + nodeToken: page1A.value.token, + newParentToken: subbookB.value.token, + newChildIndex: 0, + bookIndex: 1 // book-b + } + await manager.modifyToc(evt1) + expect(bundle.allBooks.all.toArray()[0]?.pages.size).toBe(0) + expect(bundle.allBooks.all.toArray()[1]?.pages.size).toBe(2) + subbookB = params.books[1].tocTree[0] + if (subbookB.type !== TocNodeKind.Subbook) { throw new Error('Expected subbook') } + expect(subbookB.children[0].value.token).toBe(page1A.value.token) + expect(subbookB.children[1].value.token).toBe(page1B.value.token) + expect(params.orphans).toStrictEqual([]) + + mockfs.restore() + }) it('creates a new Page in book', async () => { const book = loadSuccess(first(loadSuccess(manager.bundle).books)) expect(book.pages.size).toBe(1) const bookIndex = 0 - const { page: loadedPage } = await manager.createPage(bookIndex, 'TEST_TITLE') + const { page: loadedPage } = await manager.createPage(bookIndex, undefined, 'TEST_TITLE') expect(book.pages.size).toBe(2) expect(I.Set(book.pages).has(loadedPage)).toBe(true) expect(loadedPage.title).toBe('TEST_TITLE') // Add another page for code coverage reasons - await manager.createPage(bookIndex, 'TEST_TITLE2') + await manager.createPage(bookIndex, undefined, 'TEST_TITLE2') }) it('creates a new Subbook in book', async () => { const book = loadSuccess(first(loadSuccess(manager.bundle).books)) @@ -787,7 +837,7 @@ describe('modifyToc()', () => { expect(book.toc.length).toBe(1) const bookIndex = 0 - await manager.createSubbook(bookIndex, 'TEST_TITLE') + await manager.createSubbook(bookIndex, undefined, 'TEST_TITLE') expect(book.toc.length).toBe(2) expect(book.toc[0].type).toBe(TocNodeKind.Subbook) @@ -796,6 +846,22 @@ describe('modifyToc()', () => { expect(tocNode.title).toBe('TEST_TITLE') } else throw new Error('BUG: unreachable case') }) + + it('creates a new Ancillary in book', async () => { + const book = loadSuccess(first(loadSuccess(manager.bundle).books)) + + expect(book.pages.size).toBe(1) + + const bookIndex = 0 + const { page: loadedPage } = await manager.createAncillary(bookIndex, undefined, 'TEST_TITLE') + + expect(book.pages.size).toBe(2) + expect(I.Set(book.pages).has(loadedPage)).toBe(true) + expect(loadedPage.title).toBe('TEST_TITLE') + + // Add another page for code coverage reasons + await manager.createAncillary(bookIndex, undefined, 'TEST_TITLE2') + }) }) // ------------ Stubs ------------ diff --git a/server/src/model-manager.ts b/server/src/model-manager.ts index b1a4f111..5605dca2 100644 --- a/server/src/model-manager.ts +++ b/server/src/model-manager.ts @@ -582,6 +582,14 @@ export class ModelManager { async modifyToc(evt: TocModification) { ModelManager.debug('[MODIFY_TOC]', evt) + // !!WARNING!! + // When this function calls `writeBookToc`, it causes`this.bookTocs` to + // update with a new array, distinct from the old, before the function + // continues. This can cause variables that reference `this.bookTocs` to + // become stale which can result in unexpected behavior. For more + // information, see `sideEffectFn` in ModelManager constructor. + // !!WARNING!! + const bookToc = this.bookTocs[evt.bookIndex] const book = expectValue(this.bundle.allBooks.get(bookToc.absPath), 'BUG: Book no longer exists') const nodeAndParent = this.lookupToken(evt.nodeToken) @@ -589,7 +597,7 @@ export class ModelManager { if (nodeAndParent !== undefined) { // We are manipulating an item in a Book ToC const { node, parent } = nodeAndParent - if (evt.type === TocModificationKind.PageRename || evt.type === TocModificationKind.SubbookRename) { + if (evt.type === TocModificationKind.PageRename || evt.type === TocModificationKind.SubbookRename || evt.type === TocModificationKind.AncillaryRename) { if (node.type === TocNodeKind.Page) { const page = expectValue(this.bundle.allPages.get(node.value.absPath), `BUG: This node should exist: ${node.value.absPath}`) const fsPath = URI.parse(node.value.absPath).fsPath @@ -604,12 +612,26 @@ export class ModelManager { } else if (evt.type === TocModificationKind.Remove) { removeNode(parent, node) await writeBookToc(book, bookToc) - } else /* istanbul ignore else */ if (evt.type === TocModificationKind.Move) { + } else if (evt.type === TocModificationKind.Move) { + const recFindParentBook = (n: ClientTocNode | BookToc): BookToc => { + if (n.type === BookRootNode.Singleton) return n + const { parent } = expectValue(this.lookupToken(n.value.token), `BUG: Unexpected orphaned node: ${n.value.token}`) + return recFindParentBook(parent) + } + const srcBookToc = recFindParentBook(parent) removeNode(parent, node) // Add the node const newParentChildren = evt.newParentToken !== undefined ? childrenOf(expectValue(this.lookupToken(evt.newParentToken), 'BUG: should always have a parent').node) : bookToc.tocTree newParentChildren.splice(evt.newChildIndex, 0, node) await writeBookToc(book, bookToc) + // When moving between books in a bundle, update both collection files + if (srcBookToc.absPath !== bookToc.absPath) { + const srcBookNode = expectValue( + this.bundle.allBooks.get(srcBookToc.absPath), + `BUG: Parent book did not exist in bundle: ${srcBookToc.absPath}` + ) + await writeBookToc(srcBookNode, srcBookToc) + } } } else /* istanbul ignore else */ if (evt.type === TocModificationKind.Move) { // We are manipulating an orphaned Page (probably moving it into the ToC of a book) @@ -626,8 +648,9 @@ export class ModelManager { } } // Add the node - /* istanbul ignore next */ - const newParentChildren = evt.newParentToken !== undefined ? childrenOf(expectValue(this.lookupToken(evt.newParentToken), 'BUG: should always have a parent').node) : bookToc.tocTree + const newParentChildren = evt.newParentToken !== undefined + ? /* istanbul ignore next */ childrenOf(expectValue(this.lookupToken(evt.newParentToken), 'BUG: should always have a parent').node) + : bookToc.tocTree newParentChildren.splice(evt.newChildIndex, 0, node) await writeBookToc(book, bookToc) } else { @@ -643,7 +666,6 @@ export class ModelManager { if (node.value.token === token) { return { node, parent } } - /* istanbul ignore else */ if (node.type === TocNodeKind.Subbook) { const ret = this.recFind(token, node, node.children) if (ret !== undefined) return ret @@ -654,24 +676,11 @@ export class ModelManager { private lookupToken(token: string): Opt { for (const b of this.bookTocs) { const ret = this.recFind(token, b, b.tocTree) - /* istanbul ignore else */ if (ret !== undefined) return ret } } - public async createPage(bookIndex: number, title: string) { - const template = (): string => { - return ` - - - <metadata xmlns:md="http://cnx.rice.edu/mdml"> - <md:content-id/> - <md:uuid/> - </metadata> - <content> - </content> -</document>`.trim() - } + public async createDocument(bookIndex: number, parentNodeToken: string | undefined, title: string, documentType: string, template: string) { const workspaceRootUri = URI.parse(this.bundle.workspaceRootUri) const pageDirUri = Utils.joinPath(workspaceRootUri, 'modules') let moduleNumber = 0 @@ -686,7 +695,7 @@ export class ModelManager { const pageUri = Utils.joinPath(pageDirUri, newModuleId, 'index.cnxml') const page = this.bundle.allPages.getOrAdd(pageUri.fsPath) // fsPath works for tests and gets converted to file:// for real - const doc = new DOMParser().parseFromString(template(), 'text/xml') + const doc = new DOMParser().parseFromString(template, 'text/xml') selectOne('/cnxml:document/cnxml:title', doc).textContent = title selectOne('/cnxml:document/cnxml:metadata/md:content-id', doc).textContent = newModuleId selectOne('/cnxml:document/cnxml:metadata/md:uuid', doc).textContent = uuid4() @@ -695,23 +704,53 @@ export class ModelManager { page.load(xmlStr) await mkdirp(Utils.joinPath(pageDirUri, newModuleId).fsPath) await fs.promises.writeFile(pageUri.fsPath, xmlStr) - ModelManager.debug(`[NEW_PAGE] Created: ${pageUri.fsPath}`) + ModelManager.debug(`[NEW_${documentType.toUpperCase()}] Created: ${pageUri.fsPath}`) const bookToc = this.bookTocs[bookIndex] const book = expectValue(this.bundle.allBooks.get(bookToc.absPath), 'BUG: Book no longer exists') - bookToc.tocTree.unshift({ + /* istanbul ignore next */ + const newParentChildren = parentNodeToken !== undefined ? childrenOf(expectValue(this.lookupToken(parentNodeToken), 'BUG: should always have a parent').node) : bookToc.tocTree + newParentChildren.splice(0, 0, { type: TocNodeKind.Page, value: { token: 'unused-when-writing', title: undefined, fileId: newModuleId, absPath: page.absPath } }) await writeBookToc(book, bookToc) - ModelManager.debug(`[CREATE_PAGE] Prepended to Book: ${pageUri.fsPath}`) + ModelManager.debug(`[CREATE_${documentType.toUpperCase()}] Prepended to Book: ${pageUri.fsPath}`) return { page, id: newModuleId } } /* istanbul ignore next */ throw new Error('Error: Too many page directories already exist') } - public async createSubbook(bookIndex: number, title: string) { + public async createPage(bookIndex: number, parentToken: string | undefined, title: string) { + return await this.createDocument(bookIndex, parentToken, title, 'page', ` +<document xmlns="http://cnx.rice.edu/cnxml"> + <title/> + <metadata xmlns:md="http://cnx.rice.edu/mdml"> + <md:title/> + <md:content-id/> + <md:uuid/> + </metadata> + <content> + </content> +</document>`.trim()) + } + + public async createAncillary(bookIndex: number, parentNodeToken: string | undefined, title: string) { + return await this.createDocument(bookIndex, parentNodeToken, title, 'ancilliary', ` +<document xmlns="http://cnx.rice.edu/cnxml" class="super" resource="" document-link="" ancillary-type=""> + <title/> + <metadata xmlns:md="http://cnx.rice.edu/mdml"> + <md:title/> + <md:content-id/> + <md:uuid/> + </metadata> + <content class="super"> + </content> +</document>`.trim()) + } + + public async createSubbook(bookIndex: number, parentNodeToken: string | undefined, title: string) { ModelManager.debug(`[CREATE_SUBBOOK] Creating: ${title}`) const bookToc = this.bookTocs[bookIndex] const book = expectValue(this.bundle.allBooks.get(bookToc.absPath), 'BUG: Book no longer exists') diff --git a/server/src/model/_cli.ts b/server/src/model/_cli.ts index 2868cfce..745e6e23 100644 --- a/server/src/model/_cli.ts +++ b/server/src/model/_cli.ts @@ -229,7 +229,7 @@ function traverse(node: BookNode | TocNodeWithRange, indexes: number[]): TocPage /* Returns true if this node has nothing worth keeping (trimMe) */ function trimNodes(node: ClientTocNode | BookToc, keepPages: Set<PageNode>): boolean { - if (node.type === TocNodeKind.Page) { + if (node.type === TocNodeKind.Page || node.type === TocNodeKind.Ancillary) { const hasKeeper = [...keepPages].find(n => n.absPath === node.value.absPath) !== undefined return !hasKeeper } else { diff --git a/server/src/model/utils.ts b/server/src/model/utils.ts index 86337883..74380931 100644 --- a/server/src/model/utils.ts +++ b/server/src/model/utils.ts @@ -62,11 +62,13 @@ export function textWithRange(el: Element, attr?: string): WithRange<string> { // This also exists in ../common/ export enum TocNodeKind { Subbook = 'TocNodeKind.Subbook', - Page = 'TocNodeKind.Page' + Page = 'TocNodeKind.Page', + Ancillary = 'TocNodeKind.Ancillary' } -export type TocNode<T> = TocSubbook<T> | TocPage<T> +export type TocNode<T> = TocSubbook<T> | TocPage<T> | TocAncillary<T> export interface TocSubbook<T> { type: TocNodeKind.Subbook, readonly title: string, readonly children: Array<TocNode<T>> } export interface TocPage<T> { type: TocNodeKind.Page, readonly page: T } +export interface TocAncillary<T> { type: TocNodeKind.Ancillary, readonly page: T } export interface Paths { booksRoot: string diff --git a/server/src/server.ts b/server/src/server.ts index 1e839fca..873548ad 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -145,9 +145,11 @@ connection.onRequest(ExtensionServerRequest.TocModification, async (params: TocM const { event } = params const manager = getBundleForUri(params.workspaceUri) if (event.type === TocNodeKind.Page) { - await manager.createPage(event.bookIndex, event.title) + await manager.createPage(event.bookIndex, event.parentNodeToken, event.title) } else if (event.type === TocNodeKind.Subbook) { - await manager.createSubbook(event.bookIndex, event.title) + await manager.createSubbook(event.bookIndex, event.parentNodeToken, event.title) + } else if (event.type === TocNodeKind.Ancillary) { + await manager.createAncillary(event.bookIndex, event.parentNodeToken, event.title) } else { await manager.modifyToc(event) }