structural editing when cursor is right outside the structure

[ym-han]

The problem

Consider something like

def test():
    print("a")

and suppose the cursor is at the end of the print statement, after the ).

Currently take funk does not select the function when the cursor is at that position, though it does work if the cursor is anywhere before the final ).

edit (@pokey):

Why does this happen?

Cursorless relies on the Parse tree extension to get the current node, and then walks up from there. The problem is that this function just passes a position to web-tree-sitter's rootNode.descendantForPosition, which will always look to the right, so if there is whitespace to the right, it will go way up the tree to find a node that contains the given whitespace.

The solution

We should probably fix this one inside the parse tree extension. In getNodeAtLocation, we will proceed by trying to find an appropriate position to pass to web-tree-sitter, rather than just directly using the given position:

1. If the position is at the end of the file, subtract one from the position, unless the file is empty, in which case just use the position
2. Otherwise, inspect the character to the right of the position
3. If it is whitespace, subtract one from the position
4. Otherwise, use position

We then take the position and pass it to rootNode.descendantForPosition, like we do today

Alternate solution

If the above doesn't work for some reason, we could instead always instead first ask for the node from position like today, but then subtract one from position and ask again, and if they overlap, take the smaller one

[Will-Sommers]

Heyo, I might pick this one up. I'll put a PR up and I think perhaps add some testing infrastructure within the parse project as well.

[Will-Sommers]

I'm looking at this a bit more and am not sure that the whitespace approach will work for all languages.

In typescript for example, there is a non-whitespace statement terminator, ;(e.g. let d = [2,3,4];), or a comma , in a list or map, e.g.:

    getTree(document: vscode.TextDocument) {
...
    },

    getNodeAtLocation(location: vscode.Location) {
...
    },

These terminators/separators could be specified if we want but all of that configuration happens in the other package.

I'm going to hold off on this for a bit and think about it further. I took a cursory look at the alternate approach. If I understand it correctly, what would happen is:

1. Get the node(A) at the cursor position
2. Get the node(B) at the cursor - 1 position, find that node's parent(C) if there is one.
3. If they overlap, then choose the B, otherwise choose A.

I'm still not sure this will work in every case and in every language. I think it will work with in the case of a variable definition in Ruby, but in the case below, I don't think there is an overlap for "take funk".

function getTree(document: vscode.TextDocument) {
  return getTreeForUri(document.uri);
}|;

// position - 1
> program
>> function_declaration
>>> body: statement_block
>>>> }
>>>> "}"

// position
> program
>> empty_statement
>>> ;
>>> ";"

I guess what I'm saying is that we might need to know more about the language to be able to make a smart choice here, unless I'm thinking about this incorrectly.

Slightly unrelated, for future

Would it ever make sense to pass both down to allow for smarter inference based on the modifiers of an action? This is not a suggestion for this issue but just an observation that it might allow for more power in situations where objects are nested, such as:

a = [1,2,3]|[0]"
[a, b, [d]|]
{ a: b, c: { d: e}|}

Being able to say "take list right" or "take map right" might be pretty cool.

[pokey]

Fair point re white space. I guess the issue is

foo = "bar"|;

And you say "take value".

I think this does argue for the alternate approach, though I'm not sure if you're interpreted that one correctly

The alternative approach would happen in cursorless, rather than in parse tree extension, fwiw. Cursorless would first do its normal thing, walking up the ancestor tree from the token to the right of the cursor, yielding either a matching node or no match. But it would then move selection one left and do it's normal thing again walking from there, either getting a matching node or not.

• If only one of them matches, return it.
• If both match, and one contains the other, return the smaller.
• If neither contains the other, return the one to the right

So in your "funk" example, only the left one matches, so you'd return it, as desired. Note that if the function were nested in a bigger function, they'd both match, but the one we want is the smaller one, so we're still ok

I believe the above nicely handles the case where the input to the modifier has zero length. When the input is a range (eg "funk air"), I think the easy thing is to just walk up from the token to the right of the start of the range, like we do today.

Make sense?

[Will-Sommers]

Yep, definitely makes sense. We've got a reference to a SyntaxNode returned from vs-code-parse and then can operate on it within cursorless-vscode as we need to.

I'm likely going to operate at the processModifier wrapping the methods contained which will allow a hopefully unified behavior between identity, containingScope. It looks like surroundingPair has its own custom behavior here but this comment*" * Applies the surrounding pair modifier to the given selection. First looks to see if the target is itself adjacent to or contained by a modifier token.") makes me wonder if it could be applied there as well.

In any event, I've got a good idea about how to proceed, thanks.

[pokey]

I would start by only applying to containingScope, as I don't think it buys us anything for other modifiers, at the expense of added complexity / unpredictability

[Will-Sommers]

Just a note - #629 should fix and close this.