-
Notifications
You must be signed in to change notification settings - Fork 261
fix(ai): exit operator-pending mode if no textobject is found #1943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dd9a7ce
to
64cc9e2
Compare
The one failing test comes from the fact that the textobject is being searched twice (which will be avoided in the future), so the user is asked for input twice |
Thanks for the PR! Although this does look like fixing #1942, I am not particularly fond of the idea that every typed through mapping textobject will search twice for it. Not sure how to best handle this, though. There is also a design issue: with this PR's change the first failed application of a textobject doesn't allow dot-repeating it later (even if textobject will be found, like in another buffer). The current behavior allows to do that, which I'd expect here. But interestingly, the built-in behavior is the same as after this PR's. Maybe it is a fundamental trade-off: either allow dot-repeating later when textobject is found or resolve #1942. I'll have to think about that. |
I was fiddling with this yesterday before opening the issue and learned a few interesting things. My first approach was to try and control this in the custom operators side (because I first found this with a custom operator, not a builtin one). It seems like when the operator-pending keymap doesn't move the cursor, the Then, I tried to find ways to "cancel" an operator-pending keymap. Doing nothing inside of it's callback, still triggers the operator. Trying to simulate
Is changing the function signature of |
It will not work with dot-repeat because it needs to actually recompute the region each time the command is executed. At the time I am more leaning towards keeping this behavior as is. Mostly because it sometimes can be reasoned as expected:
Not yet 100% convinced though. More like 80% :) |
I sometimes forget how dot repeat works, sorry. Mmm, an alternative would be to
But this solution is hacky and may lead to unintuitive behaviors :p
One of the scenarios that sometimes gets me is when I want to use the replace operator from Mini.operators to put some text inside of an empty function. Since I'm using treesitter, out-of-the-box, an empty function doesn't have an inner texobject. So, the replacement gets done under the cursor. Nothing that an undo can't solve, though |
Yes, this is first way I'd think about.
Hmm... This sounds like a tree-sitter queries issue with something like "allow to match empty region". Is it literally impossible with I'll think about it more. |
After spending several sleeps on it, I think the current behavior makes more sense to me, as described in this comment. The original #1942 seems to mostly affect The fact that replacing of not found textobject still goes through is indeed unfortunate. But I think it requires a way from Vim (not even Neovim) itself to be able to "cancel" operator from inside Operator-pending mode. As far as I can see in documentation, there is currently no way to do it. I'll ask around and maybe create an issue for it. So I am going to close this. Hopefully in favor of another, more targeted solution. |
Fixes #1942. This is a naive solution that results in the textobject being searched twice. A better solution would avoid searching it multiple times, but I first wanted to know if this was a desired change and/or if modifying
select_textobject
(to optionally accept the textobject as a parameter) was an option (or if there's a better option) .