Skip to content

Commit 967a4e4

Browse files
pokeypokey
authored
Fix positional inference (#777)
* Fix positional inference * Attempt to fix ci * Update spoken forms * Cleanup * More stuff * Add return types * Cleanup * Doc string * More cleanup * Doc string * Revert test subset * Add test case for the thing we were trying to fix
1 parent b6081a2 commit 967a4e4

8 files changed

+251
-93
lines changed

‎src/core/commandRunner/CommandRunner.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as vscode from "vscode";
22
import { ActionType } from "../../actions/actions.types";
33
import { OutdatedExtensionError } from "../../errors";
44
import processTargets from "../../processTargets";
5+
import isTesting from "../../testUtil/isTesting";
56
import { Graph, ProcessedTargetsContext } from "../../typings/Types";
67
import { isString } from "../../util/type";
78
import { canonicalizeAndValidateCommand } from "../commandVersionUpgrades/canonicalizeAndValidateCommand";
@@ -146,7 +147,7 @@ export default class CommandRunner {
146147
const err = e as Error;
147148
if (err instanceof OutdatedExtensionError) {
148149
this.showUpdateExtensionErrorMessage(err);
149-
} else {
150+
} else if (!isTesting()) {
150151
vscode.window.showErrorMessage(err.message);
151152
}
152153
console.error(err.message);

‎src/core/inferFullTargets.ts

Lines changed: 102 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import {
2+
Mark,
3+
Modifier,
24
PartialListTargetDescriptor,
35
PartialPrimitiveTargetDescriptor,
46
PartialRangeTargetDescriptor,
57
PartialTargetDescriptor,
8+
PositionModifier,
69
PrimitiveTargetDescriptor,
710
RangeTargetDescriptor,
811
TargetDescriptor,
@@ -94,36 +97,30 @@ function inferPrimitiveTarget(
9497
};
9598
}
9699

97-
const hasPosition = !!target.modifiers?.find(
98-
(modifier) => modifier.type === "position"
99-
);
100+
const ownPositionalModifier = getPositionalModifier(target);
101+
const ownNonPositionalModifiers = getNonPositionalModifiers(target);
100102

101103
// Position without a mark can be something like "take air past end of line"
104+
// We will remove this case when we implement #736
102105
const mark = target.mark ??
103-
(hasPosition ? getPreviousMark(previousTargets) : null) ?? {
106+
(ownPositionalModifier == null
107+
? null
108+
: getPreviousMark(previousTargets)) ?? {
104109
type: "cursor",
105110
};
106111

107-
const previousModifiers = getPreviousModifiers(previousTargets);
112+
const nonPositionalModifiers =
113+
ownNonPositionalModifiers ??
114+
getPreviousNonPositionalModifiers(previousTargets) ??
115+
[];
108116

109-
const modifiers = target.modifiers ?? previousModifiers ?? [];
117+
const positionalModifier =
118+
ownPositionalModifier ?? getPreviousPositionalModifier(previousTargets);
110119

111-
// "bring line to after this" needs to infer line on second target
112-
const modifierTypes = [
113-
...new Set(modifiers.map((modifier) => modifier.type)),
120+
const modifiers = [
121+
...(positionalModifier == null ? [] : [positionalModifier]),
122+
...nonPositionalModifiers,
114123
];
115-
if (
116-
previousModifiers != null &&
117-
modifierTypes.length === 1 &&
118-
modifierTypes[0] === "position"
119-
) {
120-
const containingScopeModifier = previousModifiers.find(
121-
(modifier) => modifier.type === "containingScope"
122-
);
123-
if (containingScopeModifier != null) {
124-
modifiers.push(containingScopeModifier);
125-
}
126-
}
127124

128125
return {
129126
type: target.type,
@@ -132,45 +129,107 @@ function inferPrimitiveTarget(
132129
};
133130
}
134131

135-
function getPreviousMark(previousTargets: PartialTargetDescriptor[]) {
136-
return getPreviousTarget(
137-
previousTargets,
138-
(target: PartialPrimitiveTargetDescriptor) => target.mark != null
139-
)?.mark;
132+
function getPositionalModifier(
133+
target: PartialPrimitiveTargetDescriptor
134+
): PositionModifier | undefined {
135+
if (target.modifiers == null) {
136+
return undefined;
137+
}
138+
139+
const positionModifierIndex = target.modifiers.findIndex(
140+
(modifier) => modifier.type === "position"
141+
);
142+
143+
if (positionModifierIndex > 0) {
144+
throw Error("Position modifiers must be at the start of a modifier chain");
145+
}
146+
147+
return positionModifierIndex === -1
148+
? undefined
149+
: (target.modifiers[positionModifierIndex] as PositionModifier);
140150
}
141151

142-
function getPreviousModifiers(previousTargets: PartialTargetDescriptor[]) {
143-
return getPreviousTarget(
152+
/**
153+
* Return a list of non-positional modifiers on the given target. We return
154+
* undefined if there are none. Note that we will never return an empty list; we
155+
* will always return `undefined` if there are no non-positional modifiers.
156+
* @param target The target from which to get the non-positional modifiers
157+
* @returns A list of non-positional modifiers or `undefined` if there are none
158+
*/
159+
function getNonPositionalModifiers(
160+
target: PartialPrimitiveTargetDescriptor
161+
): Modifier[] | undefined {
162+
const nonPositionalModifiers = target.modifiers?.filter(
163+
(modifier) => modifier.type !== "position"
164+
);
165+
return nonPositionalModifiers == null || nonPositionalModifiers.length === 0
166+
? undefined
167+
: nonPositionalModifiers;
168+
}
169+
170+
function getPreviousMark(
171+
previousTargets: PartialTargetDescriptor[]
172+
): Mark | undefined {
173+
return getPreviousTargetAttribute(
144174
previousTargets,
145-
(target: PartialPrimitiveTargetDescriptor) => target.modifiers != null
146-
)?.modifiers;
175+
(target: PartialPrimitiveTargetDescriptor) => target.mark
176+
);
147177
}
148178

149-
function getPreviousTarget(
179+
function getPreviousNonPositionalModifiers(
180+
previousTargets: PartialTargetDescriptor[]
181+
): Modifier[] | undefined {
182+
return getPreviousTargetAttribute(previousTargets, getNonPositionalModifiers);
183+
}
184+
185+
function getPreviousPositionalModifier(
186+
previousTargets: PartialTargetDescriptor[]
187+
): PositionModifier | undefined {
188+
return getPreviousTargetAttribute(previousTargets, getPositionalModifier);
189+
}
190+
191+
/**
192+
* Walks backward through the given targets and their descendants trying to find
193+
* the first target for which the given attribute extractor returns a
194+
* non-nullish value. Returns `undefined` if none could be found
195+
* @param previousTargets The targets that precede the target we are trying to
196+
* infer. We look in these targets and their descendants for the given attribute
197+
* @param getAttribute The function used to extract the attribute from a
198+
* primitive target
199+
* @returns The extracted attribute or undefined if one could not be found
200+
*/
201+
function getPreviousTargetAttribute<T>(
150202
previousTargets: PartialTargetDescriptor[],
151-
useTarget: (target: PartialPrimitiveTargetDescriptor) => boolean
152-
): PartialPrimitiveTargetDescriptor | null {
203+
getAttribute: (target: PartialPrimitiveTargetDescriptor) => T | undefined
204+
): T | undefined {
153205
// Search from back(last) to front(first)
154206
for (let i = previousTargets.length - 1; i > -1; --i) {
155207
const target = previousTargets[i];
156208
switch (target.type) {
157-
case "primitive":
158-
if (useTarget(target)) {
159-
return target;
209+
case "primitive": {
210+
const attributeValue = getAttribute(target);
211+
if (attributeValue != null) {
212+
return attributeValue;
160213
}
161214
break;
162-
case "range":
163-
if (useTarget(target.anchor)) {
164-
return target.anchor;
215+
}
216+
case "range": {
217+
const attributeValue = getAttribute(target.anchor);
218+
if (attributeValue != null) {
219+
return attributeValue;
165220
}
166221
break;
222+
}
167223
case "list":
168-
const result = getPreviousTarget(target.elements, useTarget);
169-
if (result != null) {
170-
return result;
224+
const attributeValue = getPreviousTargetAttribute(
225+
target.elements,
226+
getAttribute
227+
);
228+
if (attributeValue != null) {
229+
return attributeValue;
171230
}
172231
break;
173232
}
174233
}
175-
return null;
234+
return undefined;
176235
}

‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndHarpToEndOfSecondCarWhaleAndEndOfWhaleTakeWhale.yml renamed to ‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndHarpToEndOfSecondCarWhaleAndEndOfJustWhaleTakeWhale.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
languageId: plaintext
22
command:
3-
version: 1
4-
spokenForm: bring point and harp to end of second car whale and end of whale take whale
5-
action: replaceWithTarget
3+
spokenForm: bring point and harp to end of second car whale and end of just whale take whale
4+
version: 2
65
targets:
76
- type: list
87
elements:
@@ -13,15 +12,22 @@ command:
1312
- type: list
1413
elements:
1514
- type: primitive
16-
position: after
17-
insideOutsideType: inside
18-
selectionType: token
19-
modifier: {type: subpiece, pieceType: character, anchor: 1, active: 1, excludeAnchor: false, excludeActive: false}
2015
mark: {type: decoratedSymbol, symbolColor: default, character: w}
16+
modifiers:
17+
- {type: position, position: end}
18+
- type: ordinalRange
19+
scopeType: {type: character}
20+
anchor: 1
21+
active: 1
22+
excludeAnchor: false
23+
excludeActive: false
2124
- type: primitive
22-
position: after
23-
insideOutsideType: inside
2425
mark: {type: decoratedSymbol, symbolColor: default, character: w}
26+
modifiers:
27+
- {type: position, position: end}
28+
- {type: toRawSelection}
29+
usePrePhraseSnapshot: false
30+
action: {name: replaceWithTarget}
2531
marksToCheck: [default.w]
2632
initialState:
2733
documentContents: hello. world

‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndHarpToStartOfSecondCarWhaleAndStartOfWhaleTakeWhale.yml renamed to ‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndHarpToStartOfSecondCarWhaleAndStartOfJustWhaleTakeWhale.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
languageId: plaintext
22
command:
3-
version: 1
43
spokenForm: >-
5-
bring point and harp to start of second car whale and start of whale take
4+
bring point and harp to start of second car whale and start of just whale take
65
whale
7-
action: replaceWithTarget
6+
version: 2
87
targets:
98
- type: list
109
elements:
@@ -15,15 +14,22 @@ command:
1514
- type: list
1615
elements:
1716
- type: primitive
18-
position: before
19-
insideOutsideType: inside
20-
selectionType: token
21-
modifier: {type: subpiece, pieceType: character, anchor: 1, active: 1, excludeAnchor: false, excludeActive: false}
2217
mark: {type: decoratedSymbol, symbolColor: default, character: w}
18+
modifiers:
19+
- {type: position, position: start}
20+
- type: ordinalRange
21+
scopeType: {type: character}
22+
anchor: 1
23+
active: 1
24+
excludeAnchor: false
25+
excludeActive: false
2326
- type: primitive
24-
position: before
25-
insideOutsideType: inside
2627
mark: {type: decoratedSymbol, symbolColor: default, character: w}
28+
modifiers:
29+
- {type: position, position: start}
30+
- {type: toRawSelection}
31+
usePrePhraseSnapshot: false
32+
action: {name: replaceWithTarget}
2733
marksToCheck: [default.w]
2834
initialState:
2935
documentContents: hello. world

‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndPointToEndOfSecondCarWhaleAndEndOfWhaleTakeWhale.yml renamed to ‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndPointToEndOfSecondCarWhaleAndEndOfJustWhaleTakeWhale.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
languageId: plaintext
22
command:
3-
version: 1
4-
spokenForm: bring point and point to end of second car whale and end of whale take whale
5-
action: replaceWithTarget
3+
spokenForm: bring point and point to end of second car whale and end of just whale take whale
4+
version: 2
65
targets:
76
- type: list
87
elements:
@@ -13,15 +12,22 @@ command:
1312
- type: list
1413
elements:
1514
- type: primitive
16-
position: after
17-
insideOutsideType: inside
18-
selectionType: token
19-
modifier: {type: subpiece, pieceType: character, anchor: 1, active: 1, excludeAnchor: false, excludeActive: false}
2015
mark: {type: decoratedSymbol, symbolColor: default, character: w}
16+
modifiers:
17+
- {type: position, position: end}
18+
- type: ordinalRange
19+
scopeType: {type: character}
20+
anchor: 1
21+
active: 1
22+
excludeAnchor: false
23+
excludeActive: false
2124
- type: primitive
22-
position: after
23-
insideOutsideType: inside
2425
mark: {type: decoratedSymbol, symbolColor: default, character: w}
26+
modifiers:
27+
- {type: position, position: end}
28+
- {type: toRawSelection}
29+
usePrePhraseSnapshot: false
30+
action: {name: replaceWithTarget}
2531
marksToCheck: [default.w]
2632
initialState:
2733
documentContents: hello. world

‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndPointToStartOfSecondCarWhaleAndStartOfWhaleTakeWhale.yml renamed to ‎src/test/suite/fixtures/recorded/hatTokenMap/bringPointAndPointToStartOfSecondCarWhaleAndStartOfJustWhaleTakeWhale.yml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
languageId: plaintext
22
command:
3-
version: 1
43
spokenForm: >-
5-
bring point and point to start of second car whale and start of whale take
4+
bring point and point to start of second car whale and start of just whale take
65
whale
7-
action: replaceWithTarget
6+
version: 2
87
targets:
98
- type: list
109
elements:
@@ -15,15 +14,22 @@ command:
1514
- type: list
1615
elements:
1716
- type: primitive
18-
position: before
19-
insideOutsideType: inside
20-
selectionType: token
21-
modifier: {type: subpiece, pieceType: character, anchor: 1, active: 1, excludeAnchor: false, excludeActive: false}
2217
mark: {type: decoratedSymbol, symbolColor: default, character: w}
18+
modifiers:
19+
- {type: position, position: start}
20+
- type: ordinalRange
21+
scopeType: {type: character}
22+
anchor: 1
23+
active: 1
24+
excludeAnchor: false
25+
excludeActive: false
2326
- type: primitive
24-
position: before
25-
insideOutsideType: inside
2627
mark: {type: decoratedSymbol, symbolColor: default, character: w}
28+
modifiers:
29+
- {type: position, position: start}
30+
- {type: toRawSelection}
31+
usePrePhraseSnapshot: false
32+
action: {name: replaceWithTarget}
2733
marksToCheck: [default.w]
2834
initialState:
2935
documentContents: hello. world

0 commit comments

Comments
 (0)