Skip to content

Conversation

TheLeoP
Copy link
Contributor

@TheLeoP TheLeoP commented Aug 15, 2025

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) .

@TheLeoP
Copy link
Contributor Author

TheLeoP commented Aug 15, 2025

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

@echasnovski
Copy link
Member

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.

@TheLeoP
Copy link
Contributor Author

TheLeoP commented Aug 15, 2025

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 ] mark will be placed in the buffer right before the [ mark (so, the end of the range is before the start). This is different than the range defined when using either of the h or l motions; in that case, both mark are placed in the same location. But, when the cursor doesn't move because it can't (like using the h motion when already in the first column of a a buffer), the range will also be inverted. Even if it were possible to distinguish a "canceled" inside of a custom operator, builtin operators would still work in the current way, so I stopped looking things in that direction.

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 <esc> with either api_nvim_feedkeys or :normal also didn't work. The only way that I managed to "cancel" an operator-pending keymap was using an <expr> keymap and returning <esc>. But, as you said, this removes the ability to dot repeat an operator-pending keymap that failed to find a textobject previously.


Not sure how to best handle this, though.

Is changing the function signature of select_textobject to receive the found (or not) text object as a parameter be an option? This would a breaking change, though.

@echasnovski
Copy link
Member

Not sure how to best handle this, though.

Is changing the function signature of select_textobject to receive the found (or not) text object as a parameter be an option? This would a breaking change, though.

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:

  • " d doesn't delete anything but clears " register" <=> "d deletes nothing and sets " to it".
  • c operator is described as "Delete {motion} text ... and start insert". So it deletes nothing and still starts Insert mode.
  • "Linewise operators like > still indent the whole line". This is the same behavior as if done something like >l, although here I'd probably expect doing nothing.

Not yet 100% convinced though. More like 80% :)

@TheLeoP
Copy link
Contributor Author

TheLeoP commented Aug 16, 2025

It will not work with dot-repeat because it needs to actually recompute the region each time the command is executed.

I sometimes forget how dot repeat works, sorry. Mmm, an alternative would be to

  • set some internal script scoped variable cached_tobj when doing the first searching
  • pass the same parameters to select_textobject, but avoid searching twice if cached_tobj is not nil
  • set cached_tobj to nil to allow dot-repeatability with different regions.

But this solution is hacky and may lead to unintuitive behaviors :p

Mostly because it sometimes can be reasoned as expected:

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

@echasnovski
Copy link
Member

Mmm, an alternative would be to

Yes, this is first way I'd think about.

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

Hmm... This sounds like a tree-sitter queries issue with something like "allow to match empty region". Is it literally impossible with vim.treesitter or are queries need adjustments? At least, that's at least one area where built-in function_call textobject is more useful :)

I'll think about it more.

@echasnovski
Copy link
Member

echasnovski commented Aug 19, 2025

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 y, as I can not reproduce it with d.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some operators are still applied even when no textobject was found
2 participants