From 4688ad8eb7ef6716138a99365df80d7e24c0256b Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 21 Jan 2025 19:05:14 +0100 Subject: [PATCH] Fix for markdown cells (#2739) The latest update for the notebook cell handler broke cells in markdown. Also implements notebook scope handler with a one of scope handler the same way we do for collection item. We can have both language specific implementations as well as the ide notebook api so this is a more proper implementation. ## Checklist - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [/] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [/] I have not broken the cheatsheet --------- Co-authored-by: Phil Cohen --- .../languages/markdown/changeCell.yml | 27 ++++ .../scopeHandlers/LineScopeHandler.ts | 2 +- .../NotebookCellApiScopeHandler.ts | 116 ++++++++++++++ .../scopeHandlers/NotebookCellScopeHandler.ts | 150 +++++------------- .../scopeHandlers/ScopeHandlerFactoryImpl.ts | 1 + 5 files changed, 189 insertions(+), 107 deletions(-) create mode 100644 data/fixtures/recorded/languages/markdown/changeCell.yml create mode 100644 packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellApiScopeHandler.ts diff --git a/data/fixtures/recorded/languages/markdown/changeCell.yml b/data/fixtures/recorded/languages/markdown/changeCell.yml new file mode 100644 index 0000000000..a166490570 --- /dev/null +++ b/data/fixtures/recorded/languages/markdown/changeCell.yml @@ -0,0 +1,27 @@ +languageId: markdown +command: + version: 7 + spokenForm: change cell + action: + name: clearAndSetSelection + target: + type: primitive + modifiers: + - type: containingScope + scopeType: {type: notebookCell} + usePrePhraseSnapshot: false +initialState: + documentContents: | + ``` + code + ``` + selections: + - anchor: {line: 1, character: 0} + active: {line: 1, character: 0} + marks: {} +finalState: + documentContents: |+ + + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/LineScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/LineScopeHandler.ts index e8e42f2ac3..da529958c5 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/LineScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/LineScopeHandler.ts @@ -15,7 +15,7 @@ export class LineScopeHandler extends BaseScopeHandler { type: "paragraph", } as const; protected readonly isHierarchical = false; - public readonly includeAdjacentInEvery: boolean = true; + public readonly includeAdjacentInEvery = true; constructor(_scopeType: ScopeType, _languageId: string) { super(); diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellApiScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellApiScopeHandler.ts new file mode 100644 index 0000000000..3260ee6c7b --- /dev/null +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellApiScopeHandler.ts @@ -0,0 +1,116 @@ +import { + Range, + type Direction, + type NotebookCell, + type Position, + type TextEditor, +} from "@cursorless/common"; +import { ide } from "../../../singletons/ide.singleton"; +import { NotebookCellTarget } from "../../targets"; +import { BaseScopeHandler } from "./BaseScopeHandler"; +import type { TargetScope } from "./scope.types"; +import type { ScopeIteratorRequirements } from "./scopeHandler.types"; + +/** + * This is the scope handler for the actual notebook API in the IDE. + */ +export class NotebookCellApiScopeHandler extends BaseScopeHandler { + public readonly scopeType = { type: "notebookCell" } as const; + public readonly iterationScopeType = { type: "document" } as const; + protected isHierarchical = false; + + constructor() { + super(); + } + + *generateScopeCandidates( + editor: TextEditor, + position: Position, + direction: Direction, + hints: ScopeIteratorRequirements, + ): Iterable { + const cells = getNotebookCells(editor, position, direction, hints); + + for (const cell of cells) { + yield createTargetScope(cell); + } + } +} + +function getNotebookCells( + editor: TextEditor, + position: Position, + direction: Direction, + hints: ScopeIteratorRequirements, +) { + const nb = getNotebook(editor); + + if (nb == null) { + return []; + } + + const { notebook, cell } = nb; + + if (hints.containment === "required") { + return [cell]; + } + + if ( + hints.containment === "disallowed" || + hints.containment === "disallowedIfStrict" + ) { + return direction === "forward" + ? notebook.cells.slice(cell.index + 1) + : notebook.cells.slice(0, cell.index).reverse(); + } + + // Every scope + if (hints.distalPosition != null) { + const searchRange = new Range(position, hints.distalPosition); + if (searchRange.isRangeEqual(editor.document.range)) { + return notebook.cells; + } + } + + return direction === "forward" + ? notebook.cells.slice(cell.index) + : notebook.cells.slice(0, cell.index + 1).reverse(); +} + +function getNotebook(editor: TextEditor) { + const uri = editor.document.uri.toString(); + for (const notebook of ide().visibleNotebookEditors) { + for (const cell of notebook.cells) { + if (cell.document.uri.toString() === uri) { + return { notebook, cell }; + } + } + } + return undefined; +} + +function createTargetScope(cell: NotebookCell): TargetScope { + const editor = getEditor(cell); + const contentRange = editor.document.range; + return { + editor, + domain: contentRange, + getTargets: (isReversed: boolean) => [ + new NotebookCellTarget({ + editor, + isReversed, + contentRange, + }), + ], + }; +} + +function getEditor(cell: NotebookCell) { + const uri = cell.document.uri.toString(); + for (const editor of ide().visibleTextEditors) { + if (editor.document.uri.toString() === uri) { + return editor; + } + } + throw new Error("Editor not found notebook cell"); +} diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellScopeHandler.ts index 5ca3859fa1..2fb820c54d 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/NotebookCellScopeHandler.ts @@ -1,132 +1,70 @@ import { - Range, type Direction, - type NotebookCell, type Position, type ScopeType, type TextEditor, } from "@cursorless/common"; import type { LanguageDefinitions } from "../../../languages/LanguageDefinitions"; -import { ide } from "../../../singletons/ide.singleton"; -import { NotebookCellTarget } from "../../targets"; +import { BaseScopeHandler } from "./BaseScopeHandler"; +import { NotebookCellApiScopeHandler } from "./NotebookCellApiScopeHandler"; +import { OneOfScopeHandler } from "./OneOfScopeHandler"; import type { TargetScope } from "./scope.types"; import type { + ComplexScopeType, ScopeHandler, ScopeIteratorRequirements, } from "./scopeHandler.types"; +import type { ScopeHandlerFactory } from "./ScopeHandlerFactory"; -export class NotebookCellScopeHandler implements ScopeHandler { +export class NotebookCellScopeHandler extends BaseScopeHandler { public readonly scopeType = { type: "notebookCell" } as const; - public readonly iterationScopeType = { type: "document" } as const; - public readonly includeAdjacentInEvery = false; + protected isHierarchical = false; + private readonly scopeHandler: ScopeHandler; - constructor( - private languageDefinitions: LanguageDefinitions, - _scopeType: ScopeType, - private languageId: string, - ) {} - - *generateScopes( - editor: TextEditor, - position: Position, - direction: Direction, - hints: ScopeIteratorRequirements, - ): Iterable { - const scopeHandler = this.languageDefinitions - .get(this.languageId) - ?.getScopeHandler(this.scopeType); - - if (scopeHandler != null) { - yield* scopeHandler.generateScopeCandidates( - editor, - position, - direction, - hints, - ); - } - - const cells = getNotebookCells(editor, position, direction, hints); - - for (const cell of cells) { - yield createTargetScope(cell); - } - } -} - -function getNotebookCells( - editor: TextEditor, - position: Position, - direction: Direction, - hints: ScopeIteratorRequirements, -) { - const nb = getNotebook(editor); - - if (nb == null) { - return []; - } - - const { notebook, cell } = nb; - - if (hints.containment === "required") { - return [cell]; + get iterationScopeType(): ScopeType | ComplexScopeType { + return this.scopeHandler.iterationScopeType; } - if ( - hints.containment === "disallowed" || - hints.containment === "disallowedIfStrict" + constructor( + scopeHandlerFactory: ScopeHandlerFactory, + languageDefinitions: LanguageDefinitions, + _scopeType: ScopeType, + languageId: string, ) { - return direction === "forward" - ? notebook.cells.slice(cell.index + 1) - : notebook.cells.slice(0, cell.index).reverse(); - } + super(); - // Every scope - if (hints.distalPosition != null) { - const searchRange = new Range(position, hints.distalPosition); - if (searchRange.isRangeEqual(editor.document.range)) { - return notebook.cells; - } - } + this.scopeHandler = (() => { + const apiScopeHandler = new NotebookCellApiScopeHandler(); - return direction === "forward" - ? notebook.cells.slice(cell.index) - : notebook.cells.slice(0, cell.index + 1).reverse(); -} + const languageScopeHandler = languageDefinitions + .get(languageId) + ?.getScopeHandler(this.scopeType); -function getNotebook(editor: TextEditor) { - const uri = editor.document.uri.toString(); - for (const notebook of ide().visibleNotebookEditors) { - for (const cell of notebook.cells) { - if (cell.document.uri.toString() === uri) { - return { notebook, cell }; + if (languageScopeHandler == null) { + return apiScopeHandler; } - } - } - return undefined; -} -function createTargetScope(cell: NotebookCell): TargetScope { - const editor = getEditor(cell); - const contentRange = editor.document.range; - return { - editor, - domain: contentRange, - getTargets: (isReversed: boolean) => [ - new NotebookCellTarget({ - editor, - isReversed, - contentRange, - }), - ], - }; -} + return OneOfScopeHandler.createFromScopeHandlers( + scopeHandlerFactory, + { + type: "oneOf", + scopeTypes: [ + languageScopeHandler.scopeType, + apiScopeHandler.scopeType, + ], + }, + [languageScopeHandler, apiScopeHandler], + languageId, + ); + })(); + } -function getEditor(cell: NotebookCell) { - const uri = cell.document.uri.toString(); - for (const editor of ide().visibleTextEditors) { - if (editor.document.uri.toString() === uri) { - return editor; - } + generateScopeCandidates( + editor: TextEditor, + position: Position, + direction: Direction, + hints: ScopeIteratorRequirements, + ): Iterable { + return this.scopeHandler.generateScopes(editor, position, direction, hints); } - throw new Error("Editor not found notebook cell"); } diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts index 2626555830..50152f21ef 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ScopeHandlerFactoryImpl.ts @@ -114,6 +114,7 @@ export class ScopeHandlerFactoryImpl implements ScopeHandlerFactory { ); case "notebookCell": return new NotebookCellScopeHandler( + this, this.languageDefinitions, scopeType, languageId,