feat: Improve operator-pending mode behavior

[vanaigr]

Checklist

• [x] Used only camelCase variable names.
• [ ] If functionality is added or modified, also made respective changes to the README.md.

---

Currently, the plugin switches to visual mode when selecting the last character in the line in operator-pending mode. Since selection option affects whether the last character in visual mode is included, setting selection=exclusive breaks the motion:

vim.cmd([[set selection=exclusive]])
vim.keymap.set('o', 'e', function() require('spider').motion('e') end)

For this text (|a| means that the cursor is on the character 'a'):

|a|bc

typing de would not delete the last character 'c'.

This PR fixes this; de deletes from the current position to the end of the word regardless of whether selection is inclusive or exclusive.

---

Using v to change inclusiveness also doesn't affect e, i.e. de is equivalent to dve. This PR makes dve exclusive (since de is inclusive), i.e. the end character is not selected.

---

I wrote a post describing various quirks of how motions work in (neo)vim. One of them is that if the end position of an exclusive charwise motion can never be at the start of the line, i.e. currently, for this text:

  a|b|c
def

typing dw would not delete the EOL between the 2 lines:

  |a|
def

But if the cursor is moved one char to the left:

  |a|bc
def

then dw would delete the entire first line (including indentation before the cursor), since the motion is updated to be linewise:

|d|ef

  Both of these are different from the base case (both words on the same line):

  a|b|c def

where dw results in this:

  a|d|ef

  This PR makes dw in the first case result in this:

  a|d|ef

and in the second case:

  |d|ef

I.e. the w always acts as the base case, even if the end position happens to be at the start of the line.

---

I wrote a plugin that handles these cases and would like to know what would be the best way to integrate it into this plugin. Its code can just copied into the project, or It can be added to the config as an optional dependency.

[chrisgrieser]

Thanks for the PR! I like that you not only try to improve stuff practically, but also try to think about how the vim motions work on a more "theoretical" level.

---

So, a few things/questions about this:

• I didn't even know selection existed as a setting, interesting. How much of the issues you describe would be solved by temporarily changing selection during spider's operation?
• "I wrote a post describing various quirks of how motions work in (neo)vim." Yeah, I also stumbled on some quirks, for instance the inconsistency between cw and dw. I personally prefer predictable, consistent motions over motions that copy vim's inconsistencies. It's not fully clear to me what the PR/your plugin does in this regard – does it fix/remove those vim quirks? Or does it make spider behave more like original vim motions, essentially "adding" the quirks? If it's the former, I'm in favor of simply integrating your plugin into spider, if it's the latter, I think I would tend to go for an optional dependency for those who prefer that behavior.
• Either way, I think the change is significant enough to warrant a short section under the paragraph "special cases" in the Readme.
• I think a three-way comparison between original vim, nvim-spider, and your PR might be needed to properly evaluate how we proceed with this. From your examples, it's not entirely clear to me which ones exactly you are comparing. Also, by using | to signify the cursor, it's not fully clear to me where the cursor is positioned in your examples – could you maybe do something like this to make it unambiguously clear where the cursor is positioned?

  local foo = barbar
  ------- ^

• (In a somewhat related manner, having a bunch of those examples would probably be useful as test cases, for when I finally find the time to add proper tests to this plugin...)

[vanaigr]

How much of the issues you describe would be solved by temporarily changing selection during spider's operation?

Sadly, none, since changing the option doesn't adjust the cursor position (apart from selection=old), only the meaning of the endpoints.

I personally prefer predictable, consistent motions over motions that copy vim's inconsistencies. It's not fully clear to me what the PR/your plugin does in this regard – does it fix/remove those vim quirks?

Yes, the idea of the plugin is to facilitate defining consistent text objects. The plugin handles all the different cases, so that regardless of the user options, the operated region is the same.

Also, by using | to signify the cursor, it's not fully clear to me where the cursor is positioned in your examples – could you maybe do something like this to make it unambiguously clear where the cursor is positioned?

I adjusted the examples. The character under the cursor is now enclosed by |.

[chrisgrieser]

Ah yes, I understand it now. Vim even switching the word motion to a linewise motion is really a weird one. I agree, a PR fixing stuff like that is desirable.

In this case, I think we can go the "fully integrate route":

• [ ] you can add the motion module to this PR
• [ ] I think the module should be renamed to something like operater-pending or something, since calling it motion would be a bit confusing.
• [ ] since the PR is quite large then already, leave out the tests you have at your plugin (you are welcome to add them in a separate PR once we finish this one!)
• [ ] add a sub-section to the README, where you briefly summarize these changes. For consistency of the README, it would be great if you could also use ----^ to indicate cursor locations, since that is how the other examples in the README do it.
• [ ] nvim-spider uses camelCase for variables, while your plugin uses snake_case. Could you switch the variables to camelCase for consistency? (iirc, text-case can speed up bulk-case-conversion quite a bit)
• [ ] given the size of this PR, I think we should make the changes of the PR opt-in at first, in case there are unforeseen edge cases or weird interactions with custom pattern etc. Basically, to be enabled via a setting in the setup call. When things prove to be stable, we can make it the default behavior.

[vanaigr]

I added the files under lua/spider/operator-pending, and added a subsection in the readme.

I also added the consistentOperatorPending flag that enables the feature.

[chrisgrieser]

Thank you! nice work