search

cursorless-dev / cursorless

Don't let the cursor slow you down
https://www.cursorless.org/
MIT License
1.14k stars 79 forks source link

Change "that" target for "chuck" to be untyped #2475

Closed pokey closed 3 months ago

pokey commented 3 months ago

This was an issue raised by a keyboard user. The problem is that "chuck line air" followed by "pour that" resulted in just moving cursor to the start of the line after the one that was deleted. I think this change is fairly harmless

I kept it as having an explicit range because that way "chuck air" followed by "take that" won't select the adjacent token. However, keep in mind that this means that "chuck air" followed by "take every token that" won't select every token on that line.

I think prob the way forward is to revisit the auto-expansion, but for now this solves the problem of "pour that" after "chuck line air" or "chuck air"

I also don't feel super strongly here; happy to bail on this PR if we don't like the semantics

Checklist

AndreasArvidsson commented 3 months ago

I wouldn't say I'm against it, but I'm not just sure what I would want "chuck line air pour that" to actually do. First I thought it would insert a new line where the old one was, but it actually jumps over the next line. ie if you chuck the fifth line and then pour that you now get a new line after what would use to be the sixth line. I just don't know what is the right thing to do or if it's even usable. I'm almost tempted to not have a that mark on chuck. In all honesty the target is gone?

pokey commented 3 months ago

Huh yeah interesting point. Tbh after thinking some more I wonder if the problem here is that in keyboard mode we set keyboard mark to that mark after an action. Maybe instead we should just set it to the current cursor position

AndreasArvidsson commented 3 months ago

Huh yeah interesting point. Tbh after thinking some more I wonder if the problem here is that in keyboard mode we set keyboard mark to that mark after an action. Maybe instead we should just set it to the current cursor position

Yeah that sounds reasonable.

pokey commented 3 months ago

closing in favour of https://github.com/cursorless-dev/cursorless/pull/2529