TSServer: Smart select API

[mjbvz]

Background

VS Code has proposed a smart selection API (microsoft/vscode#63935) that allows language extensions to intelligently control how selections are expanded and collapsed.

Just as an example, consider a simple class:

class Foo {
  bar(a, b) {
    if (a === b) {
      return true
    }
    return false;
  } 
}

If the user starts with their cursor in the a in a === b expression, progressively running expand selection may result in the following selections:

1. The identifier a
2. The expression a === b
3. The body of the bar method
4. The entire bar method declaration
5. The entire Foo class

The exact selections we want may differ.

You can find the current state of the VS Code API proposal in vscode.proposed.d.ts under the name selection range provider

Proposal

I think we should investigate supporting smart select for JavaScript and TypeScript users, but do so in a cross editor compatible way. The current proposal is derived from VS Code's needs so we should make sure it will work for other editors too

The current VS Code smart selection API proposal takes a position in a document and returns a list of potential expansion ranges. Each returned range also has some metadata about the range's kind (class, expression, statement, interface, ...)

On the TS Server side, a possible API would be:

interface SelectionRangeRequest extends FileLocationRequest {
     command: "selectionRanges";
     arguments: SelectionRangeRequestArgs;
}

interface SelectionRangeRequestArgs extends FileLocationRequestArgs { }

interface SelectionRangeResponse {
     body?: ReadOnlyArray<SelectionRange>;
}

interface SelectionRange {
    textSpan: TextSpan;
    kind: SelectionKind;
}

// Either enum or string?
// VS Code has proposed a dot separated scope: expression.identifier, statement.if
enum SelectionKind {
    Expression,
    Statement,
    Class
}

We also need to determine which selection ranges we should target for a first iteration. A few basic ones:

• Whole expression
• Statement line
• Class / interface
• Function
• Literal

/cc @DanielRosenwasser, @jrieken, @amcasey

[jrieken]

related LSP discussion microsoft/language-server-protocol#613

[amcasey]

/cc @minestarks

[minestarks]

A FileRangeRequestArgs might be more appropriate than FileLocationRequestArgs, so that you can pass in the currently selected span.

I think in VS this would roughly map to an implementation of GetSpanOfEnclosing . I don't see any causes for concern with the proposed command / response schema.

[mjbvz]

Here's some thoughts on how expansion could work:

Where should selection stops be added?

Walk the ast from the current position up to the root node, adding a selection to the smart select response when encountering:

• Expressions
• Statements
• Type annotation (such as param: number)
• Object literal element (key + value)
• Block bodies (both for lists of statements and for class/interface/object bodies, but without the braces)
• Comments
• String content (without the quotes)
• Lists of imports
• Whole program

Examples

class Foo {
  bar(a, b) {
    if (a === b) {
      return true;
    }
    return false;
  } 
}

Starting on true in return true

• true
• return true;
• The body of the if statement (without braces)
• The entire if statement
• The body of bar (without braces)
• The entire bar method
• The body of the class (without braces)
• The entire class declaration

---

---

class Foo {
  bar(a, b) {
    if (a === b) {
      return true
    }
    return false;
  } 
}

Starting on a in a === b

• a
• a === b
• The if statement
• The body of bar (without braces)
• The entire bar method
• The body of the class (without braces)
• The entire class declaration
• whole program

---

export interface IService {
	_serviceBrand: any;

	open(host: number, data: any): Promise<any>;
}

Starting on host

• host
• host: number
• host: number, data: any
• open(host: number, data: any): Promise<any>;
• the body of IService (without braces)
• the IService interface
• whole program

---

import { x as y, z } from './z';
import { b } from './';

console.log(1);

Starting on x in x as y

• x
• x as y
• x as y, z
• { x as y, z }
• import { x as y, z } from './z';
• the list of imports
• whole program

[mjbvz]

We've refined our API on the VS Code side and have decided not to include the Kind in v1. We've also moved to use a linked list structure instead of an array. Here's an update proposal that reflects this:

interface SelectionRangeRequest extends FileLocationRequest {
     command: "selectionRange";
     arguments: SelectionRangeRequestArgs;
}

interface SelectionRangeRequestArgs extends FileLocationRequestArgs { }

interface SelectionRangeResponse {
     body?: SelectionRange;
}

interface SelectionRange {
    textSpan: TextSpan;
    parent?: SelectionRange; 
}

The VS Code api also takes a list of positions in the file and we return a list of selection ranges for each of those positions. This avoids extra requests in some cases. We should consider doing this in the TS api as well, as I don't think there's any downside. This would look like:

interface SelectionRangeRequest extends FileLocationRequest {
     command: "selectionRange";
     arguments: SelectionRangeRequestArgs;
}

interface SelectionRangeRequestArgs extends FileRequestArgs {
    locations: Location[];
}

interface SelectionRangeResponse {
     body?: SelectionRange[];
}

interface SelectionRange {
    textSpan: TextSpan;
    parent?: SelectionRange; 
}

Here's the VS Code api: https://github.com/microsoft/vscode/blob/caafa5b6920b0e4b8ce2a8300a61568372da0f7b/src/vs/vscode.d.ts#L3791

/cc @jrieken

[andrewbranch]

@mjbvz when you mention that blocks should be selected without braces, do you want a) the selection to be begin and end at non-whitespace characters, or do you want b) all characters including whitespace between the braces to be selected?

a	b

[mjbvz]

B I think. That's how we've implemented the feature for VS Code's json and html support at least

[andrewbranch]

Can I get your input on these?

type X = { /*|*/x?: string };

type M = { [K in keyof /*|*/any]: string };

const x = { /*|*/y };

function f(...ar/*|*/gs) {}

function f(
  /*|*/a,
  b,
) {}

const s = `one two ${
  /*|*/three
} four`;

Also, I was looking at an existing smart select test in VS Code, and it looks like there are cases where you expect to get a range with an identical parent range? My approach so far includes deduping identical ranges (e.g., in a whole file containing a single function declaration with no surrounding trivia, the function declaration range is the same as the whole file range) on TypeScript's side. Let me know if you'd rather me keep syntactically distinct but positionally redundant ranges in the list.

[mjbvz]

De-duplicating the ranges sounds correct to me. @jrieken can you confirm if smart select providers should de-duplicate or not?

Some thoughts on those examples:

type X = { /*|*/x?: string };

• x
• x?
• x?: string
• { x?: string }
• type X = { x?: string };

---

type M = { [K in keyof /*|*/any]: string };

• any
• K in keyof any
• [K in keyof any]
• [K in keyof any]: string
• { [K in keyof any]: string }
• type M = { [K in keyof any]: string };

---

const x = { /*|*/y };

• y
• { y }
• const x = { y };

---

function f(...ar/*|*/gs) {}

• args
• ...args
• function f(...args) {}

---

function f(
  /*|*/a,
  b,
) {}

• a

a,
  b,

function f(
  /*|*/a,
  b,
) {}

---

const s = `one two ${
  /*|*/three
} four`;

• three

${
  /*|*/three
}

one two ${
  /*|*/three
} four

`one two ${
  /*|*/three
} four`

const s = `one two ${
  /*|*/three
} four`;

[jrieken]

@jrieken can you confirm if smart select providers should de-duplicate or not?

It's kind of optional because the editor will deduplicate ranges anyways - so if it hinders an elegant implementation I won't do it.

[andrewbranch]

@mjbvz @jrieken thanks! So, in my earlier question, I had swapped my "a" and "b" labels such that the screenshots demonstrated the opposite of the options I had written, but I assumed when you responded "B," you were referring to the picture. (I've since edited the question to be consistent.)

If your answer is still "B," I'm trying to figure out the logical rule for: when do we select all the way up to braces vs. leaving trivia unselected? E.g., in the if statement we want to select all the way up to the braces, but in the import clause ({ x as y, z }) and most of the object-like syntaxes I sent you, we only select up to the whitespace.

I've been trying to think through these scenarios in terms of the probable desired outcomes a user has when expanding a selection, and how easy it is to achieve those outcomes given a particular selection. When braces are on the same line, it's a bit of a toss-up:

I want a multi-line object	Backspace, Backspace, Delete, Return	Return
I want to start my single-line object over	(just start typing)	Space, Space, Left Arrow
I want {}	Backspace, Backspace, Delete	Backspace

When the braces span multiple lines, selecting all the way up to the braces seems to be more versatile:

I want a multi-line object type	(just start typing)	Return
I want a single-line object type	Backspace, Cmd+Backspace, Backspace, Space, Delete, Delete?	Space, Space, Left Arrow
I want {}	Same as above plus one more Delete	Backspace

Now, the really painful sequences above can be avoided by simply expanding the selection one more time to include the braces, then typing {. However, for class declaration bodies and function parameters, that’s not an option:

I want to rewrite my parameters on multiple lines	(just start typing)	Return
I want to delete my parameters or rewrite them on a single line	(painful)	Backspace

These scenarios lead me to suggest one of two possible governing rules:

1. Always add a stop on both whitespace-exclusive contents and whitespace-inclusive contents. Most versatile, but maybe users would feel like it's too much granularity?
2. Stop only on whitespace-inclusive contents when braces or parens are on separate lines; stop only on whitespace-exclusive contents when braces or parens are on the same line. This eliminates the most painful scenarios without introducing extra stops. I also think it matches your intuition on every example you've given me so far except the function with parameters on separate lines, which I would select all the way up to the parens.

There are possibly other heuristics that could be considered (like the difference between objects, which have a stop just outside the braces, and classes, which select the braces plus the text class ClassName ), but that might be too unintuitive.

By the way, I'm in Redmond (visiting from SF) until Friday afternoon, so if it's easier to discuss in person, feel free to ping me or come by the TypeScript team room.

[andrewbranch]

Updating thread with results of in-person meeting for posterity: we agreed to go with option 2 above and see how it feels once we have it wired up with VS Code.

[mjbvz]

One other selection stop to consider for multiline cases: select the entire line including leading whitespace.

For code such as:

function foo(
   /**/a,
   b    
)

This would result in the selections:

[mjbvz]

I've checked in a stubbed out VS Code implementation with microsoft/vscode@f635233

This is based on the second version of the api proposal, the one that takes a Location[] and returns a SelectionRange[]

[andrewbranch]

Cool, it works with my WIP branch!

It does look like the ranges provided by tsserver are getting merged with Code's other guesses, e.g. when on Syn/**/taxKind, I verified that tsserver says “select SyntaxKind,” but the first selection is just Syntax. Not sure if this is intentional or not, guess that's up to you all.

[jrieken]

but the first selection is just Syntax. Not sure if this is intentional or not, guess that's up to you all.

Yeah, that's VSCode itself, expanding words along separators, like aA,_, or -

[andrewbranch]

Ok @mjbvz @jrieken, do you want to give it a try? I'm getting the feeling that I could find little bits of syntax to nitpick and customize forever, but I'm pretty sure at this stage I've covered every example listed in this issue discussion plus some. If you don't find any glaring issues, I'll open a PR. Thanks!

[mjbvz]

Thanks @andrewbranch. I've done some initial testing and ran into one problem. With the cursor at the /**/ in the code:

const z = 1;

interface foo {
    bar(): void/**/
}

In this case, the first returned range is:

        "textSpan": {
            "start": {
                "line": 4,
                "offset": 9
            },
            "end": {
                "line": 4,
                "offset": 9
            }
        },

which causes VS Code's selection range conversion to fail. The top returned range must contain the initial position.

I'll keep testing and share other feedback

[mjbvz]

Looking good overall. A few other notes:

---

const callback/**/ = this.fetchCallback(serverRequest.seq);

There's currently a stop where just the assignment expression is selected:

I don't think this step is necessary. We could jump right to selecting the entire const statement

---

Doc comments are currently not handled. I think these docs should either be included when the element being documented is or as a separate step. For example:

const x = 1;

/**
 * docs
 */
function foo/**/() {}

Would have some stop with the selection:

---

Invalid selection range also returned for the case:

function foo() {
	// bla/**/
}

[mihailik]

Few more cases may need to consider with explicit separate rules:

• JSX <elements attributes={express + ions}> and static text inside those </elements>
• `string literals with ${expres + sions} inside them`
• Generic<Ty|P|E, "Arguments">

[mihailik]

Also, should this enrich resulting range objects with minimal context description?

When IDE extends selection, imagine a tooltip fade in for few seconds, indicating WHAT is selected:

Could be helpful in complex code/JSON spanning multiple screen heights.

[andrewbranch]

@mjbvz @jrieken I just found a case where adding VS Code interlacing its own expansions (specifically the one where it wants to select a full line) produces a pretty weird sequence:

You can see the expansion where [K in keyof P]: actually gets removed, which I think should never be the case when expanding. TypeScript is giving instructions to expand through the whole type on the right-hand side of the colon, but once that type spans multiple lines, I guess VS Code wants the whole line to be highlighted first. But that line taken on its own is semantically nonsensical.

I would suggest for VS Code to:

1. disable these full-line selections when taking ranges from TSServer, or
2. disable them when they are outside the range you get from TSServer.

[andrewbranch]

Simpler example:

[andrewbranch]

I have a general fix for this in vscode that prevents any line selection range that isn’t fully contained by the next range in the list from being added. Will add a test after lunch and PR.