cursorless icon indicating copy to clipboard operation
cursorless copied to clipboard

Update scope type modifier to expand on both ends

Open AndreasArvidsson opened this issue 3 years ago • 2 comments

Right now you can take lines paragraphs and soon non whitespace sequences that expands from both anchor and active of the selection. Currently containing scopes only use start off the selection.

I want both of these to have the same result. (air and bat are in different functions) take funk air past bat take air past bat take funk

I was thinking that we could implement this first thing in the selection/scope modifier by basically calling the specific implementation twice, once for both ends. To day we pass the entire selection to the specific modifier(line, funk, ...). What if we pass in a single position instead?

const startSelection = getSelectionForScope(selection.start, "function"); // function is of course a variable
if (selection.isEmpty() 
  || getNodeAtLocation(selection.start).id === getNodeAtLocation(selection.end).id) {
    return startSelection;
}
const endSelection = getSelectionForScope(selection.end, "function");
return  getSelectionFromPositions(startSelection.start, endSelection.end);
  • [ ] Add test for https://github.com/pokey/cursorless-vscode/issues/484#issuecomment-1013856057
  • [ ] Add test for "funk wrap that" with that mark starting in one statement and ending in another (see https://github.com/cursorless-dev/cursorless/issues/883)
  • [ ] Add test for "snip funk after that" with that mark starting in one function and ending in another (see https://github.com/cursorless-dev/cursorless/issues/883)
  • [ ] https://github.com/cursorless-dev/cursorless/issues/901

edit (@pokey)

Let's implement this one in a new ContainingScope stage that all containing scopes share. The algorithm should be as described in https://github.com/cursorless-dev/cursorless/pull/629#issuecomment-1136090441. The initial implementation will be folded into https://github.com/cursorless-dev/cursorless/pull/629

AndreasArvidsson avatar Jan 15 '22 20:01 AndreasArvidsson

I don't think this will work for "round", even if we decide to make it consistent with "funk". The problem is if the user has (()), and says "take round repper", referring to the first ). When expanding from the end, it will look right and end up with the whole string, which is incorrect

update: the algorithm described in https://github.com/cursorless-dev/cursorless/pull/629#issuecomment-1136090441 should handle this case just fine, because it will first try the start of the target, which is in the center of all the parens, and the returned domain will be (), which weakly contains the end of the target, so it will just return it

pokey avatar Jan 16 '22 11:01 pokey

Yes some special handling of matching pairs is fine. Just getting all the containing scope and selection types working as one would be nice.

AndreasArvidsson avatar Jan 16 '22 11:01 AndreasArvidsson