GeezerVison is showing github.com — TSServer: Smart select API · Issue #29071 · microsoft/TypeScript
TSServer: Smart select API #29071DescriptionBackground
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:
- The identifier
a
- The expression
a === b
- The body of the
bar method
- The entire
bar method declaration
- 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 Daniel Rosenwasser (@DanielRosenwasser), Johannes Rieken (@jrieken), Andrew Casey (@amcasey) Reacted by Marin Marinov, Matt Bierner, Thomas Cranny and Jihoon Lee Reacted by Jiri Spac, Kris Kaczor, Josh Ghoulberg 👻, Ádám Lippai, ZpdDG4gta and Alex Ware Activityjrieken commented on Dec 18, 2018 amcasey commented on Jan 2, 2019 minestarks commented on Jan 3, 2019 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 commented on Apr 4, 2019 ContributorAuthor More actions 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
truereturn 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
aa === 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: numberhost: number, data: anyopen(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
xx as yx as y, z{ x as y, z }import { x as y, z } from './z';- the list of imports
- whole program
mjbvz commented on Apr 5, 2019 ContributorAuthor More actions 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 Johannes Rieken (@jrieken) andrewbranch commented on Apr 8, 2019 Matt Bierner (@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 commented on Apr 9, 2019 ContributorAuthor More actions B I think. That's how we've implemented the feature for VS Code's json and html support at least andrewbranch commented on Apr 9, 2019 Can I get your input on these?
type X = { /*|*/x?: string };type M = { [K in keyof /*|*/any]: string };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. andrewbranch commented on Apr 11, 2019 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 commented on Apr 11, 2019 ContributorAuthor More actions One other selection stop to consider for multiline cases: select the entire line including leading whitespace.
For code such as:
This would result in the selections:
mjbvz commented on Apr 11, 2019 ContributorAuthor More actions 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 commented on Apr 12, 2019 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 commented on Apr 15, 2019
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 commented on Apr 16, 2019 Ok Matt Bierner (@mjbvz) Johannes Rieken (@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 commented on Apr 17, 2019 ContributorAuthor More actions Thanks Andrew Branch (@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 commented on Apr 17, 2019 ContributorAuthor More actions 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 commented on May 1, 2019 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 commented on May 1, 2019 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 commented on May 16, 2019 Matt Bierner (@mjbvz) Johannes Rieken (@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:
- disable these full-line selections when taking ranges from TSServer, or
- disable them when they are outside the range you get from TSServer.
andrewbranch commented on May 16, 2019 Simpler example:
andrewbranch commented on May 16, 2019 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.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
