Skip to content

Bugfixes: past token, past end of, subtoken out of range, sort tokens #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Aug 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 72 additions & 5 deletions src/NavigationMap.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { TextDocumentChangeEvent, Range } from "vscode";
import { TextDocumentChangeEvent, Range, TextDocument } from "vscode";
import { SymbolColor } from "./constants";
import { Token } from "./Types";
import { selectionWithEditorFromPositions } from "./selectionUtils";
import { SelectionWithEditor, Token } from "./Types";

/**
* Maps from (color, character) pairs to tokens
Expand Down Expand Up @@ -64,9 +65,75 @@ export default class NavigationMap {
this.map = {};
}

public getTokenForRange(range: Range) {
return Object.values(this.map).find(
(token) => token.range.intersection(range) != null
/**
* Given a selection returns a new selection which contains the tokens
* intersecting the given selection. Uses heuristics to tie break when the
* given selection is empty and abuts 2 adjacent tokens
* @param selection Selection to operate on
* @returns Modified selection
*/
public getTokenSelectionForSelection(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code doesn't really fit in the navigation map. Earmarking to move it for cursorless engine 2.0 work, as it should become a proper scope type

selection: SelectionWithEditor
): SelectionWithEditor | null {
const range = selection.selection;
const tokens = range.isEmpty
? this.getTokensForEmptyRange(selection.editor.document, range)
: this.getTokensForRange(selection.editor.document, range);
if (tokens.length < 1) {
return null;
}
const start = tokens[0].range.start;
const end = tokens[tokens.length - 1].range.end;
return selectionWithEditorFromPositions(selection, start, end);
}

// Return tokens for overlapping ranges
private getTokensForRange(document: TextDocument, range: Range) {
const tokens = Object.values(this.map).filter((token) => {
if (token.editor.document !== document) {
return false;
}
const intersection = token.range.intersection(range);
return intersection != null && !intersection.isEmpty;
});
tokens.sort((a, b) => a.startOffset - b.startOffset);
return tokens;
}

// Returned single token for overlapping or adjacent range
private getTokensForEmptyRange(document: TextDocument, range: Range) {
const tokens = Object.values(this.map).filter(
(token) =>
token.editor.document === document &&
token.range.intersection(range) != null
);

// If multiple matches sort and take the first
tokens.sort((a, b) => {
// First sort on alphanumeric
const aIsAlphaNum = isAlphaNum(a.text);
const bIsAlphaNum = isAlphaNum(b.text);
if (aIsAlphaNum && !bIsAlphaNum) {
return -1;
}
if (bIsAlphaNum && !aIsAlphaNum) {
return 1;
}

// Second sort on length
const lengthDiff = b.text.length - a.text.length;
if (lengthDiff !== 0) {
return lengthDiff;
}

// Lastly sort on start position. ie leftmost
return a.startOffset - b.startOffset;
});

return tokens.slice(0, 1);
}
}

function isAlphaNum(text: string) {
return /^\w+$/.test(text);
}
2 changes: 1 addition & 1 deletion src/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export type Delimiter =
| "backtickQuotes";

export type ScopeType =
| "attribute"
| "argumentOrParameter"
| "arrowFunction"
| "class"
Expand All @@ -90,7 +91,6 @@ export type ScopeType =
| "string"
| "type"
| "value"
| "xmlAttribute"
| "xmlElement"
| "xmlBothTags"
| "xmlEndTag"
Expand Down
2 changes: 1 addition & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export async function activate(context: vscode.ExtensionContext) {
} else {
if (await testCaseRecorder.start()) {
vscode.window.showInformationMessage(
"Recording test cases for following commands"
`Recording test cases for following commands in:\n${testCaseRecorder.fixtureSubdirectory}`
);
}
}
Expand Down
19 changes: 13 additions & 6 deletions src/inferFullTargets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,10 @@ function inferRangeStartTarget(
prototypeTargets: Target[],
actionPreferences: ActionPreferences
): PrimitiveTarget {
const mark = target.mark ?? CURSOR_MARK;
const mark =
target.mark ??
(target.selectionType === "token" ? CURSOR_MARK_TOKEN : CURSOR_MARK);

prototypeTargets = hasContent(target) ? [] : prototypeTargets;

const selectionType =
Expand Down Expand Up @@ -375,13 +378,17 @@ export function inferRangeEndTarget(
? []
: possiblePrototypeTargetsIncludingStartTarget;

const startMark = extractAttributeFromList(
possiblePrototypeTargetsIncludingStartTarget,
"mark"
);

const mark =
target.mark ??
extractAttributeFromList(
possiblePrototypeTargetsIncludingStartTarget,
"mark"
) ??
CURSOR_MARK;
(startMark != null && startMark.type !== CURSOR_MARK.type
? startMark
: null) ??
(target.selectionType === "token" ? CURSOR_MARK_TOKEN : CURSOR_MARK);

const selectionType =
target.selectionType ??
Expand Down
2 changes: 1 addition & 1 deletion src/languages/cpp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const nodeMatchers: Partial<Record<ScopeType, NodeMatcherAlternative>> = {
value: valueMatcher("*[declarator][value]", "*[value]", "assignment_expression[right]", "optional_parameter_declaration[default_value]"),
collectionItem: argumentMatcher("initializer_list"),
argumentOrParameter: argumentMatcher("parameter_list", "argument_list"),
xmlAttribute: "attribute"
attribute: "attribute"
};

export default createPatternMatchers(nodeMatchers);
2 changes: 1 addition & 1 deletion src/languages/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ const nodeMatchers: Partial<Record<ScopeType, NodeMatcherAlternative>> = {
),
argumentOrParameter: argumentMatcher("formal_parameters", "arguments"),
// XML, JSX
xmlAttribute: ["jsx_attribute"],
attribute: ["jsx_attribute"],
xmlElement: ["jsx_element", "jsx_self_closing_element"],
xmlBothTags: getTags,
xmlStartTag: getStartTag,
Expand Down
89 changes: 53 additions & 36 deletions src/processTargets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
SelectionWithEditor,
Target,
TypedSelection,
Position as TargetPosition,
InsideOutsideType,
} from "./Types";

export default function processTargets(
Expand Down Expand Up @@ -199,19 +201,15 @@ function getSelectionsFromMark(
return context.sourceMark;

case "cursorToken": {
const tokens = context.currentSelections.map((selection) => {
const token = context.navigationMap.getTokenForRange(
selection.selection
);
if (token == null) {
throw new Error("Couldn't find mark under cursor");
const tokenSelections = context.currentSelections.map((selection) => {
const tokenSelection =
context.navigationMap.getTokenSelectionForSelection(selection);
if (tokenSelection == null) {
throw new Error("Couldn't find token in selection");
}
return token;
return tokenSelection;
});
return tokens.map((token) => ({
selection: new Selection(token.range.start, token.range.end),
editor: token.editor,
}));
return tokenSelections;
}

case "decoratedSymbol":
Expand Down Expand Up @@ -334,6 +332,15 @@ function transformSelection(
const activeIndex =
modifier.active < 0 ? modifier.active + pieces.length : modifier.active;

if (
anchorIndex < 0 ||
activeIndex < 0 ||
anchorIndex >= pieces.length ||
activeIndex >= pieces.length
) {
throw new Error("Subtoken index out of range");
}

const isReversed = activeIndex < anchorIndex;

const anchor = selection.selection.start.translate(
Expand Down Expand Up @@ -461,6 +468,8 @@ function createTypedSelection(
selectionContext: getTokenSelectionContext(
selection,
modifier,
position,
insideOutsideType,
selectionContext
),
};
Expand Down Expand Up @@ -600,6 +609,8 @@ function performPositionAdjustment(
function getTokenSelectionContext(
selection: SelectionWithEditor,
modifier: Modifier,
position: TargetPosition,
insideOutsideType: InsideOutsideType,
selectionContext: SelectionContext
): SelectionContext {
if (!isSelectionContextEmpty(selectionContext)) {
Expand All @@ -611,32 +622,38 @@ function getTokenSelectionContext(

const document = selection.editor.document;
const { start, end } = selection.selection;

const startLine = document.lineAt(start);
const leadingText = startLine.text.slice(0, start.character);
const leadingDelimiters = leadingText.match(/\s+$/);
const leadingDelimiterRange =
leadingDelimiters != null
? new Range(
start.line,
start.character - leadingDelimiters[0].length,
start.line,
start.character
)
: null;

const endLine = document.lineAt(end);
const trailingText = endLine.text.slice(end.character);
const trailingDelimiters = trailingText.match(/^\s+/);
const trailingDelimiterRange =
trailingDelimiters != null
? new Range(
end.line,
end.character,
end.line,
end.character + trailingDelimiters[0].length
)
: null;
let leadingDelimiterRange, trailingDelimiterRange;

// Position start/end of has no delimiter
if (position !== "before" || insideOutsideType !== "inside") {
const startLine = document.lineAt(start);
const leadingText = startLine.text.slice(0, start.character);
const leadingDelimiters = leadingText.match(/\s+$/);
leadingDelimiterRange =
leadingDelimiters != null
? new Range(
start.line,
start.character - leadingDelimiters[0].length,
start.line,
start.character
)
: null;
}

if (position !== "after" || insideOutsideType !== "inside") {
const trailingText = endLine.text.slice(end.character);
const trailingDelimiters = trailingText.match(/^\s+/);
trailingDelimiterRange =
trailingDelimiters != null
? new Range(
end.line,
end.character,
end.line,
end.character + trailingDelimiters[0].length
)
: null;
}

const isInDelimitedList =
(leadingDelimiterRange != null || trailingDelimiterRange != null) &&
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
spokenForm: take past end of token
languageId: typescript
command:
actionName: setSelection
partialTargets:
- type: range
start: {type: primitive}
end: {type: primitive, position: after, insideOutsideType: inside, selectionType: token}
excludeStart: false
excludeEnd: false
extraArgs: []
marks: {}
initialState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 8}
finalState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 11}
thatMark:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 11}
fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: after, modifier: {type: identity}, insideOutsideType: inside}}]
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
spokenForm: take past start of token
languageId: typescript
command:
actionName: setSelection
partialTargets:
- type: range
start: {type: primitive}
end: {type: primitive, position: before, insideOutsideType: inside, selectionType: token}
excludeStart: false
excludeEnd: false
extraArgs: []
marks: {}
initialState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 8}
finalState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 6}
thatMark:
- anchor: {line: 0, character: 8}
active: {line: 0, character: 6}
fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: cursorToken}, selectionType: token, position: before, modifier: {type: identity}, insideOutsideType: inside}}]
31 changes: 31 additions & 0 deletions src/test/suite/fixtures/recorded/compoundTargets/takePastTrap.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
spokenForm: take past trap
languageId: typescript
command:
actionName: setSelection
partialTargets:
- type: range
start: {type: primitive}
end:
type: primitive
mark: {type: decoratedSymbol, symbolColor: default, character: t}
excludeStart: false
excludeEnd: false
extraArgs: []
marks:
default.t:
start: {line: 0, character: 6}
end: {line: 0, character: 11}
initialState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 3}
active: {line: 0, character: 3}
finalState:
documentContents: hello there
selections:
- anchor: {line: 0, character: 3}
active: {line: 0, character: 11}
thatMark:
- anchor: {line: 0, character: 3}
active: {line: 0, character: 11}
fullTargets: [{type: range, excludeStart: false, excludeEnd: false, start: {type: primitive, mark: {type: cursor}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}, end: {type: primitive, mark: {type: decoratedSymbol, symbolColor: default, character: t}, selectionType: token, position: contents, modifier: {type: identity}, insideOutsideType: inside}}]
Loading