Skip to content

Commit b1f084e

Browse files
AndreasArvidssonpokey
AndreasArvidsson
and
pokey
authored
Make token removal range less aggressive (#1564)
Fixes #1077 ## 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: Pokey Rule <[email protected]>
1 parent 6076d86 commit b1f084e

File tree

19 files changed

+387
-62
lines changed

19 files changed

+387
-62
lines changed

‎packages/cursorless-engine/src/processTargets/targetUtil/insertionRemovalBehaviors/TokenInsertionRemovalBehavior.ts

Lines changed: 53 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
import { Range, TextDocument, TextEditor } from "@cursorless/common";
2-
import { tokenize } from "../../../tokenizer";
1+
import { Range, TextEditor } from "@cursorless/common";
32
import type { Target } from "../../../typings/target.types";
43
import { expandToFullLine } from "../../../util/rangeUtils";
54
import { PlainTarget } from "../../targets";
65

6+
const leadingDelimiters = ['"', "'", "(", "[", "{", "<"];
7+
const trailingDelimiters = ['"', "'", ")", "]", "}", ">", ",", ";", ":"];
8+
79
export function getTokenLeadingDelimiterTarget(
810
target: Target,
911
): Target | undefined {
@@ -53,10 +55,27 @@ export function getTokenTrailingDelimiterTarget(
5355
}
5456

5557
/**
56-
* Constructs a removal range for the given target that will clean up a json
57-
* whitespace on one side unless it will cause two tokens to be merged. This
58-
* removal range is designed to be used with things that should clean themselves
59-
* up as if they're a range of tokens.
58+
* Constructs a removal range for the given target that may remove whitespace on
59+
* one side. This removal range is designed to be used with things that should
60+
* clean themselves up as if they're a range of tokens.
61+
*
62+
* We determine whether to include adjacent whitespace based on the following
63+
* rules:
64+
*
65+
* - If we would just be leaving a line with whitespace on it, we delete the
66+
* whitespace
67+
* - Otherwise, if there is trailing whitespace, we include it if any of the
68+
* following is true:
69+
* - there is leading whitespace, OR
70+
* - we are at start of line, OR
71+
* - there is an approved leading delimiter character (eg `(`, `[`, etc).
72+
* - Otherwise, if there is leading whitespace, we include it if any of the
73+
* following is true:
74+
* - we are at end of line, OR
75+
* - there is an approved trailing delimiter character (eg `)`, `]`, `:`, `;`,
76+
* etc).
77+
* - Otherwise, we don't include any adjacent whitespace
78+
*
6079
* @param target The target to get the token removal range for
6180
* @returns The removal range for the given target
6281
*/
@@ -82,70 +101,45 @@ export function getTokenRemovalRange(target: Target): Range {
82101
return fullLineRange;
83102
}
84103

104+
// Use trailing range if: There is a leading range OR there is no leading
105+
// content OR there is an approved leading delimiter character
85106
if (!trailingWhitespaceRange.isEmpty) {
86-
const candidateRemovalRange = contentRange.union(trailingWhitespaceRange);
87-
88-
if (!mergesTokens(editor, contentRange, candidateRemovalRange)) {
89-
// If there is trailing whitespace and it doesn't result in tokens getting
90-
// merged, then we remove it
91-
return candidateRemovalRange;
107+
if (
108+
!leadingWhitespaceRange.isEmpty ||
109+
contentRange.start.isEqual(fullLineRange.start) ||
110+
leadingDelimiters.includes(getLeadingCharacter(editor, contentRange))
111+
) {
112+
return contentRange.union(trailingWhitespaceRange);
92113
}
93114
}
94115

95-
if (
96-
!leadingWhitespaceRange.isEmpty &&
97-
leadingWhitespaceRange.start.character !== 0
98-
) {
99-
const candidateRemovalRange = leadingWhitespaceRange.union(contentRange);
100-
101-
if (!mergesTokens(editor, contentRange, candidateRemovalRange)) {
102-
// If there is leading whitespace that is not indentation and it doesn't
103-
// result in tokens getting merged, then we remove it
104-
return candidateRemovalRange;
116+
// Use leading range if: There is no trailing content OR there is an approved
117+
// trailing delimiter character
118+
if (!leadingWhitespaceRange.isEmpty) {
119+
if (
120+
contentRange.end.isEqual(fullLineRange.end) ||
121+
trailingDelimiters.includes(getTrailingCharacter(editor, contentRange))
122+
) {
123+
return contentRange.union(leadingWhitespaceRange);
105124
}
106125
}
107126

108127
// Otherwise just return the content range
109128
return contentRange;
110129
}
111130

112-
/** Returns true if removal range causes tokens to merge */
113-
function mergesTokens(
114-
editor: TextEditor,
115-
contentRange: Range,
116-
removalRange: Range,
117-
) {
118-
const { document } = editor;
119-
const fullRange = expandToFullLine(editor, contentRange);
120-
const fullText = document.getText(fullRange);
121-
const fullTextOffset = document.offsetAt(fullRange.start);
122-
123-
const numTokensContentRangeRemoved = calculateNumberOfTokensAfterRemoval(
124-
document,
125-
fullText,
126-
fullTextOffset,
127-
contentRange,
128-
);
129-
130-
const numTokensRemovalRangeRemoved = calculateNumberOfTokensAfterRemoval(
131-
document,
132-
fullText,
133-
fullTextOffset,
134-
removalRange,
135-
);
136-
137-
return numTokensContentRangeRemoved !== numTokensRemovalRangeRemoved;
131+
function getLeadingCharacter(editor: TextEditor, contentRange: Range): string {
132+
const { start } = contentRange;
133+
const line = editor.document.lineAt(start);
134+
return start.isAfter(line.range.start)
135+
? editor.document.getText(new Range(start.translate(undefined, -1), start))
136+
: "";
138137
}
139138

140-
function calculateNumberOfTokensAfterRemoval(
141-
document: TextDocument,
142-
fullText: string,
143-
fullTextOffset: number,
144-
removalRange: Range,
145-
): number {
146-
const startIndex = document.offsetAt(removalRange.start) - fullTextOffset;
147-
const endIndex = document.offsetAt(removalRange.end) - fullTextOffset;
148-
const modifiedText = fullText.slice(0, startIndex) + fullText.slice(endIndex);
149-
const tokens = tokenize(modifiedText, document.languageId, (m) => m);
150-
return tokens.length;
139+
function getTrailingCharacter(editor: TextEditor, contentRange: Range): string {
140+
const { end } = contentRange;
141+
const line = editor.document.lineAt(end);
142+
return end.isBefore(line.range.end)
143+
? editor.document.getText(new Range(end.translate(undefined, 1), end))
144+
: "";
151145
}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/rust/chuckFunkNameFine.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ initialState:
2121
end: {line: 0, character: 2}
2222
finalState:
2323
documentContents: |-
24-
fn() {
24+
fn () {
2525
println!("Hello, world!");
2626
}
2727
selections:

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/languages/rust/chuckName2.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ initialState:
2020
marks: {}
2121
finalState:
2222
documentContents: |
23-
fn() -> Result<(), Error> {
23+
fn () -> Result<(), Error> {
2424
2525
}
2626
selections:

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/positions/chuckAir.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ finalState:
2424
"hello "
2525
" hello"
2626
" hello "
27-
return.b
27+
return .b
2828
selections:
2929
- anchor: {line: 4, character: 0}
3030
active: {line: 4, character: 0}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckBat.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck bat
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
mark: {type: decoratedSymbol, symbolColor: default, character: b}
9+
usePrePhraseSnapshot: true
10+
initialState:
11+
documentContents: "aaa bbb: ccc"
12+
selections:
13+
- anchor: {line: 0, character: 12}
14+
active: {line: 0, character: 12}
15+
marks:
16+
default.b:
17+
start: {line: 0, character: 4}
18+
end: {line: 0, character: 7}
19+
finalState:
20+
documentContents: "aaa: ccc"
21+
selections:
22+
- anchor: {line: 0, character: 8}
23+
active: {line: 0, character: 8}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: foo = bar.baz
14+
selections:
15+
- anchor: {line: 0, character: 6}
16+
active: {line: 0, character: 10}
17+
marks: {}
18+
finalState:
19+
documentContents: foo = baz
20+
selections:
21+
- anchor: {line: 0, character: 6}
22+
active: {line: 0, character: 6}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken10.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: "\"a b c\""
14+
selections:
15+
- anchor: {line: 0, character: 5}
16+
active: {line: 0, character: 5}
17+
marks: {}
18+
finalState:
19+
documentContents: "\"a b\""
20+
selections:
21+
- anchor: {line: 0, character: 4}
22+
active: {line: 0, character: 4}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken11.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: a.b c
14+
selections:
15+
- anchor: {line: 0, character: 2}
16+
active: {line: 0, character: 2}
17+
marks: {}
18+
finalState:
19+
documentContents: a. c
20+
selections:
21+
- anchor: {line: 0, character: 2}
22+
active: {line: 0, character: 2}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken12.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: a! -b
14+
selections:
15+
- anchor: {line: 0, character: 2}
16+
active: {line: 0, character: 2}
17+
marks: {}
18+
finalState:
19+
documentContents: a -b
20+
selections:
21+
- anchor: {line: 0, character: 1}
22+
active: {line: 0, character: 1}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken13.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: a b
14+
selections:
15+
- anchor: {line: 0, character: 0}
16+
active: {line: 0, character: 0}
17+
marks: {}
18+
finalState:
19+
documentContents: b
20+
selections:
21+
- anchor: {line: 0, character: 0}
22+
active: {line: 0, character: 0}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken14.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: a b
14+
selections:
15+
- anchor: {line: 0, character: 2}
16+
active: {line: 0, character: 2}
17+
marks: {}
18+
finalState:
19+
documentContents: a
20+
selections:
21+
- anchor: {line: 0, character: 1}
22+
active: {line: 0, character: 1}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken2.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: "\"foo-bar bongo-bazman\""
14+
selections:
15+
- anchor: {line: 0, character: 9}
16+
active: {line: 0, character: 21}
17+
marks: {}
18+
finalState:
19+
documentContents: "\"foo-bar\""
20+
selections:
21+
- anchor: {line: 0, character: 8}
22+
active: {line: 0, character: 8}

‎packages/cursorless-vscode-e2e/src/suite/fixtures/recorded/scopes/token/chuckToken3.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
languageId: plaintext
2+
command:
3+
version: 5
4+
spokenForm: chuck token
5+
action: {name: remove}
6+
targets:
7+
- type: primitive
8+
modifiers:
9+
- type: containingScope
10+
scopeType: {type: token}
11+
usePrePhraseSnapshot: true
12+
initialState:
13+
documentContents: hello there,
14+
selections:
15+
- anchor: {line: 0, character: 6}
16+
active: {line: 0, character: 6}
17+
marks: {}
18+
finalState:
19+
documentContents: hello,
20+
selections:
21+
- anchor: {line: 0, character: 5}
22+
active: {line: 0, character: 5}

0 commit comments

Comments
 (0)