From e58d7173b8e19e9db8007ba3edc73827ff437e87 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Thu, 22 Jun 2023 13:14:42 +0200 Subject: [PATCH 1/3] Fixing b}gs on items scope --- .../modifiers/ItemStage/getIterationScope.ts | 68 +++++++++++++++---- .../recorded/itemTextual/clearItem12.yml | 22 ++++++ .../recorded/itemTextual/clearItem13.yml | 22 ++++++ .../recorded/itemTextual/clearItem14.yml | 22 ++++++ 4 files changed, 120 insertions(+), 14 deletions(-) create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem12.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem13.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem14.yml diff --git a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts index 6c845fef24..52bcee3608 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts @@ -20,25 +20,16 @@ export function getIterationScope( // Iteration is necessary in case of nested strings while (surroundingTarget != null) { - const surroundingStringTarget = getStringSurroundingPair( - languageDefinitions, - surroundingTarget, - ); - - // We don't look for items inside strings. if ( - // Not in a string - surroundingStringTarget == null || - // In a non-string surrounding pair that is inside a surrounding string. This is fine. - surroundingStringTarget.contentRange.start.isBefore( - surroundingTarget.contentRange.start, + useInteriorOfSurroundingTarget( + languageDefinitions, + target, + surroundingTarget, ) ) { return { range: surroundingTarget.getInteriorStrict()[0].contentRange, - boundary: surroundingTarget - .getBoundaryStrict() - .map((t) => t.contentRange) as [Range, Range], + boundary: getBoundary(surroundingTarget), }; } @@ -55,6 +46,55 @@ export function getIterationScope( }; } +function useInteriorOfSurroundingTarget( + languageDefinitions: LanguageDefinitions, + target: Target, + surroundingTarget: SurroundingPairTarget, +): boolean { + const { contentRange } = target; + + if (contentRange.isEmpty) { + // const [left, right] = getBoundary(surroundingTarget); + // const line = target.editor.document.lineAt(contentRange.start); + // TODO: if content range is adjacent to boundary and nothing else. Return false + } else { + // Content range is equal to surrounding range + if (contentRange.isRangeEqual(surroundingTarget.contentRange)) { + return false; + } + + // Content range is equal to one of the boundaries of the surrounding range + const [left, right] = getBoundary(surroundingTarget); + if (contentRange.isRangeEqual(left) || contentRange.isRangeEqual(right)) { + return false; + } + } + + // We don't look for items inside strings. + // A non-string surrounding pair that is inside a surrounding string is fine. + const surroundingStringTarget = getStringSurroundingPair( + languageDefinitions, + surroundingTarget, + ); + if ( + surroundingStringTarget != null && + surroundingTarget.contentRange.start.isBeforeOrEqual( + surroundingStringTarget.contentRange.start, + ) + ) { + return false; + } + + return true; +} + +function getBoundary(surroundingTarget: SurroundingPairTarget): [Range, Range] { + return surroundingTarget.getBoundaryStrict().map((t) => t.contentRange) as [ + Range, + Range, + ]; +} + function getParentSurroundingPair( languageDefinitions: LanguageDefinitions, editor: TextEditor, diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem12.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem12.yml new file mode 100644 index 0000000000..c7182e41e1 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem12.yml @@ -0,0 +1,22 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: "[1, [2, 3]];" + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 10} + marks: {} +finalState: + documentContents: "[1, ];" + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem13.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem13.yml new file mode 100644 index 0000000000..ea1bfdd233 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem13.yml @@ -0,0 +1,22 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: "[1, [2, 3]];" + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 5} + marks: {} +finalState: + documentContents: "[1, ];" + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem14.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem14.yml new file mode 100644 index 0000000000..e42aeb01ea --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem14.yml @@ -0,0 +1,22 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: "[1, [2, 3]];" + selections: + - anchor: {line: 0, character: 10} + active: {line: 0, character: 9} + marks: {} +finalState: + documentContents: "[1, ];" + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} From 3ee60cb9eecc030b91a7c800e19ef2cf7be9dd2e Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 23 Jun 2023 05:13:02 +0200 Subject: [PATCH 2/3] Don't use the interior of matching pairs when adjacent to delimiter --- .../modifiers/ItemStage/getIterationScope.ts | 37 +++++++++++++++++-- .../recorded/itemTextual/chuckEveryItem.yml | 11 +++--- .../recorded/itemTextual/clearItem15.yml | 33 +++++++++++++++++ .../recorded/itemTextual/clearItem16.yml | 33 +++++++++++++++++ .../recorded/itemTextual/clearItem17.yml | 33 +++++++++++++++++ .../recorded/itemTextual/clearItem18.yml | 33 +++++++++++++++++ .../languages/typescript/takeItem.yml | 4 +- 7 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem15.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem16.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem17.yml create mode 100644 packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem18.yml diff --git a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts index 52bcee3608..127ae69f6c 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts @@ -1,4 +1,4 @@ -import { Range, TextEditor } from "@cursorless/common"; +import { Range, TextEditor, TextLine } from "@cursorless/common"; import { LanguageDefinitions } from "../../../languages/LanguageDefinitions"; import { Target } from "../../../typings/target.types"; import { PlainTarget, SurroundingPairTarget } from "../../targets"; @@ -54,9 +54,27 @@ function useInteriorOfSurroundingTarget( const { contentRange } = target; if (contentRange.isEmpty) { - // const [left, right] = getBoundary(surroundingTarget); - // const line = target.editor.document.lineAt(contentRange.start); - // TODO: if content range is adjacent to boundary and nothing else. Return false + const [left, right] = getBoundary(surroundingTarget); + const pos = contentRange.start; + // Content range is outside adjacent to pair + if (pos.isEqual(left.start) || pos.isEqual(right.end)) { + return false; + } + const line = target.editor.document.lineAt(pos); + // Content range is just inside of opening/left delimiter + if ( + pos.isEqual(left.end) && + characterIsWhitespaceOrMissing(line, pos.character) + ) { + return false; + } + // Content range is just inside of closing/right delimiter + if ( + pos.isEqual(right.start) && + characterIsWhitespaceOrMissing(line, pos.character - 1) + ) { + return false; + } } else { // Content range is equal to surrounding range if (contentRange.isRangeEqual(surroundingTarget.contentRange)) { @@ -95,6 +113,17 @@ function getBoundary(surroundingTarget: SurroundingPairTarget): [Range, Range] { ]; } +function characterIsWhitespaceOrMissing( + line: TextLine, + index: number, +): boolean { + return ( + index < line.range.start.character || + index >= line.range.end.character || + line.text[index].trim() === "" + ); +} + function getParentSurroundingPair( languageDefinitions: LanguageDefinitions, editor: TextEditor, diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml index c984703a14..cd88435760 100644 --- a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml @@ -20,11 +20,12 @@ initialState: active: {line: 0, character: 1} marks: {} finalState: - documentContents: |- - { - + documentContents: |2- + + "hello": "there", + "testing": "whatever", } selections: - - anchor: {line: 0, character: 1} - active: {line: 0, character: 1} + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} fullTargets: [{type: primitive, mark: {type: cursor}, modifiers: [{type: everyScope, scopeType: {type: collectionItem}}]}] diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem15.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem15.yml new file mode 100644 index 0000000000..0c477c41c8 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem15.yml @@ -0,0 +1,33 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: |- + [ + [ + 1, + 2 + ], + 3 + ]; + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} + marks: {} +finalState: + documentContents: |- + [ + , + 3 + ]; + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem16.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem16.yml new file mode 100644 index 0000000000..1a6d8bb1b2 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem16.yml @@ -0,0 +1,33 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: |- + [ + [ + 1, + 2 + ], + 3 + ]; + selections: + - anchor: {line: 1, character: 5} + active: {line: 1, character: 5} + marks: {} +finalState: + documentContents: |- + [ + , + 3 + ]; + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem17.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem17.yml new file mode 100644 index 0000000000..2f588ebb11 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem17.yml @@ -0,0 +1,33 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: |- + [ + [ + 1, + 2 + ], + 3 + ]; + selections: + - anchor: {line: 4, character: 4} + active: {line: 4, character: 4} + marks: {} +finalState: + documentContents: |- + [ + , + 3 + ]; + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem18.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem18.yml new file mode 100644 index 0000000000..8e027bf232 --- /dev/null +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/clearItem18.yml @@ -0,0 +1,33 @@ +languageId: plaintext +command: + version: 5 + spokenForm: clear item + action: {name: clearAndSetSelection} + targets: + - type: primitive + modifiers: + - type: containingScope + scopeType: {type: collectionItem} + usePrePhraseSnapshot: true +initialState: + documentContents: |- + [ + [ + 1, + 2 + ], + 3 + ]; + selections: + - anchor: {line: 4, character: 5} + active: {line: 4, character: 5} + marks: {} +finalState: + documentContents: |- + [ + , + 3 + ]; + selections: + - anchor: {line: 1, character: 4} + active: {line: 1, character: 4} diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/typescript/takeItem.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/typescript/takeItem.yml index d3a8550375..6d83a9b99c 100644 --- a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/typescript/takeItem.yml +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/typescript/takeItem.yml @@ -19,6 +19,6 @@ finalState: const value = { a: 1, b: 2, c: 3 }; selections: - - anchor: {line: 1, character: 16} - active: {line: 1, character: 20} + - anchor: {line: 1, character: 0} + active: {line: 1, character: 35} fullTargets: [{type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: containingScope, scopeType: collectionItem}, insideOutsideType: inside}] From ce515fbbcfb07efb813c6789db5a2be387742828 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Fri, 23 Jun 2023 09:22:44 +0200 Subject: [PATCH 3/3] updated test --- .../modifiers/ItemStage/getIterationScope.ts | 2 +- .../recorded/itemTextual/chuckEveryItem.yml | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts index 127ae69f6c..28a1b4e7af 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/ItemStage/getIterationScope.ts @@ -18,7 +18,7 @@ export function getIterationScope( ): { range: Range; boundary?: [Range, Range] } { let surroundingTarget = getSurroundingPair(languageDefinitions, target); - // Iteration is necessary in case of nested strings + // Iteration is necessary in case of in valid surrounding targets (nested strings, content range adjacent to delimiter) while (surroundingTarget != null) { if ( useInteriorOfSurroundingTarget( diff --git a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml index cd88435760..ced954c609 100644 --- a/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml +++ b/packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/itemTextual/chuckEveryItem.yml @@ -16,16 +16,15 @@ initialState: "testing": "whatever", } selections: - - anchor: {line: 0, character: 1} - active: {line: 0, character: 1} + - anchor: {line: 1, character: 1} + active: {line: 1, character: 1} marks: {} finalState: - documentContents: |2- - - "hello": "there", - "testing": "whatever", + documentContents: |- + { + } selections: - - anchor: {line: 0, character: 0} - active: {line: 0, character: 0} + - anchor: {line: 1, character: 1} + active: {line: 1, character: 1} fullTargets: [{type: primitive, mark: {type: cursor}, modifiers: [{type: everyScope, scopeType: {type: collectionItem}}]}]