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

* Support take token past

* Support take past token

* Added more tests

* Better error message for sub token index out of range

* No delimiters on start and end of positions

* Sort multiple match tokens

* Sort tokens on length

* Sort tokens on length

* Sort on alphanumeric as well

* Added additional tests

* Change directory

* Add directory to a message

* Changed inference on end

* Added tests

* find tokens by range

* Renamed attributes

* Add docstring

* Fix multi-editor bug

Co-authored-by: Pokey Rule <[email protected]>

Author: AndreasArvidsson

src/NavigationMap.ts

@@ -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
@@ -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(
+    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);
+}

src/Types.ts

@@ -71,6 +71,7 @@ export type Delimiter =
   | "backtickQuotes";
 
 export type ScopeType =
+  | "attribute"
   | "argumentOrParameter"
   | "arrowFunction"
   | "class"
@@ -90,7 +91,6 @@ export type ScopeType =
   | "string"
   | "type"
   | "value"
-  | "xmlAttribute"
   | "xmlElement"
   | "xmlBothTags"
   | "xmlEndTag"

src/extension.ts

@@ -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}`
           );
         }
       }

src/inferFullTargets.ts

@@ -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 =
@@ -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 ??

src/languages/cpp.ts

@@ -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);

src/languages/typescript.ts

@@ -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,

src/processTargets.ts

@@ -24,6 +24,8 @@ import {
   SelectionWithEditor,
   Target,
   TypedSelection,
+  Position as TargetPosition,
+  InsideOutsideType,
 } from "./Types";
 
 export default function processTargets(
@@ -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":
@@ -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(
@@ -461,6 +468,8 @@ function createTypedSelection(
         selectionContext: getTokenSelectionContext(
           selection,
           modifier,
+          position,
+          insideOutsideType,
           selectionContext
         ),
       };
@@ -600,6 +609,8 @@ function performPositionAdjustment(
 function getTokenSelectionContext(
   selection: SelectionWithEditor,
   modifier: Modifier,
+  position: TargetPosition,
+  insideOutsideType: InsideOutsideType,
   selectionContext: SelectionContext
 ): SelectionContext {
   if (!isSelectionContextEmpty(selectionContext)) {
@@ -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) &&

src/test/suite/fixtures/recorded/compoundTargets/takePastEndOfToken.yml

@@ -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}}]

src/test/suite/fixtures/recorded/compoundTargets/takePastStartOfToken.yml

@@ -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}}]

src/test/suite/fixtures/recorded/compoundTargets/takePastTrap.yml

@@ -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}}]