Feat: add support for dressing.nvim

[smjonas]

Hey @RaafatTurki , I have added support for dressing.nvim! Could you please help me test out this PR before I merge it? I want to make sure I didn't break anything. You can use it by specifiying the dressing_support branch in your package manager.

To use this feature, you simply have to call :lua require("inc_rename").rename() while dressing.nvim is installed (or any other plugin that uses a separate buffer for entering the new name).

[RaafatTurki]

I've faced two major bugs so far, tested on sumneko (lua) and gopls (go) and in both cases I've made a:

• side file defining a sub() function
• main file defining an add() function and calling both add() and sub() The idea is to test renaming within the same file add() and across files sub()

1. sumneko

   • renaming add() worked flawlessly
   • renaming sub() worked with a side effect, it has inserted the changed line into main as well (and in the same position as if it was the side file)

2. gopls

   • renaming add() previews correctly on each keypress within the input box but it reverts back to the old name upon pressing enter
   • renaming sub() has the same issue as add() and also introduces the same side effect from sub() in sumneko

[smjonas]

Thanks for the feedback, very useful! I also noticed some issues that need to be fixed before merging.

[smjonas]

Should be ready now, please test again :D I had to slightly change the behavior when an empty name is entered because otherwise the buffer restoration became too difficult, but I don't think this is a big deal (it can be still be reimplemented in the future, maybe as an option).

Note to self: there's still one (rare) bug that needs to be fixed (rename and WinClosed autocommand are triggered both)

[RaafatTurki]

You've outdone yourself, this is fantastic! It even works with splits.

Since you've squashed both major bugs I've went ahead and tested this with sumneko_lua, gopls, clangd, tsserver, pyright hence all the issues below are nitpicks/polish and tbh I don't know if all of them directly relate to this branch so take everything with a pinch of salt.

- pressing escape mid-renaming results in unwanted behavior

I believe this is a simple matter of blocking the rename request on abort, which is solved by nil checking new_name right above this line since nil in that context means input was aborted according to docs gopls: clangd: pyright: tsserver: function gets renamed to undefined

- input box shows even if what's under the cursor not renamable (even in whitespace or on a blank line)

- there's no way to prefill the word under the cursor like in the old implementation

A little tip

override col and row in the dressing setup() like so to show the input box right above the cursor (not covering the word being renamed)

  require 'dressing'.setup {
    input = {
      enabled = true,
      override = function(conf)
        conf.col = -1
        conf.row = 0
        return conf
      end,
    },
  }

[smjonas]

Thank you very much for your awesome feedback again, really appreciate it!

pressing escape mid-renaming results in unwanted behavior

Strangely, I could not reproduce this myself but I applied the change you suggested - makes sense.

there's no way to prefill the word under the cursor like in the old implementation

You can achieve this by doing the following (I changed the method signature to accept a table now instead of the default name only, this way the interface can be extended in the future more easily): require("inc_rename").rename({ default = vim.fn.expand("<cword>") }) I have also updated the readme to reflect these changes.

override col and row in the dressing setup() like so to show the input box right above the cursor (not covering the word being renamed)

Definitely looks nicer, I have added your tip to the readme.

input box shows even if what's under the cursor not renamable (even in whitespace or on a blank line)

Great suggestion, I'll probably do that in a separate commit!

[RaafatTurki]

Thank you very much for your awesome feedback again, really appreciate it!

No thank you for your quick edits and progress in general!

Strangely, I could not reproduce this myself but I applied the change you suggested - makes sense

It's fixed on my end now, Perhaps you've got something else blocking the rename request on your neovim setup.

this way the interface can be extended in the future more easily

It works nicely, I like the table approach as well.

Definitely looks nicer, I have added your tip to the readme

Awesome!

Great suggestion, I'll probably do that in a separate commit

Yeah I figured that one effects both implementations.

Note to self: there's still one (rare) bug that needs to be fixed (rename and WinClosed autocommand are triggered both)

I couldn't really understand when this actually happens, If it's still there and you need me to test it let me know.

Other than that It seems like this PR is close to its merging point, I'll report back if I find any more bugs in the future.

[smjonas]

I couldn't really understand when this actually happens, If it's still there and you need me to test it let me know.

This sometimes happen when renaming across multiple files (only for renaming across a buffer, not for the :IncRename command). See this screen recording:

https://user-images.githubusercontent.com/40792180/176206071-00dc1853-e0ed-46ff-8152-e3eecf5bd5bb.mp4

Notice how it mostly works except during the last rename operation where fn on the right-hand side is not renamed to fn. This bug seems to occur when the lines in the buffer are reset right before the rename operation so the LSP server only finds one instance in one file. So the underlying problem is that this autocommand https://github.com/smjonas/inc-rename.nvim/blob/74ddfd0dd9a8dac8f645668fbaa857b80cd2381f/lua/inc_rename/init.lua#L355-L362 is triggered even though we don't want it to trigger - instead only the rename command (the vim.ui callback) should be executed:

https://github.com/smjonas/inc-rename.nvim/blob/74ddfd0dd9a8dac8f645668fbaa857b80cd2381f/lua/inc_rename/init.lua#L367-L370

Here are some additional details about this bug if you want to know more :) The problem is that the WinClosed autocommand always runs before the vim.ui callback. I don't know how to either cancel this autocommand when the user hits enter in the input box or schedule the closing autocommand after the rename command (this way it can be detected that it should return early). Using vim.schedule inside the highlight function does schedule it after the callback - no idea why. I had hoped that there is some other autocommand event that works but did not find any, they are all executed before the callback.

Any ideas are very welcome here :D

[smjonas]

since nil in that context means input was aborted according to docs

I think your comment from earlier just gave me an idea... I think based on this we can avoid the autocommand altogether :tada:

[RaafatTurki]

Annnd the autocmd is gone, hahah

[smjonas]

Now it seems to work perfectly with dressing.nvim, but the default rename command does weird highlighting things now... :smile:

[smjonas]

Everything seems to work fine now, I'd appreciate one last test from you before merging :)

[RaafatTurki]

A major issue was introduced with this last commit:

First an error shows up on rename confirm: gopls

clangd

sumneko_lua Doesn't show an error but it's notorious for being silent, The rest still applies.

The renamed parts are still highlighted as if we're still in the input box. And whenever a word search is done (using / or ?) or when executing :normal! : all the renamed parts revert to the old name.

The only way I've found to persist the renames was to restart neovim right after renaming something.

The same applies when renaming across files The same applies even when aborting the input box

[smjonas]

Thanks a lot for helping me, I should have tested it with more language servers, it was working fine with sumneko_lua. I now tested it with clangd and I did not encounter any issues anymore (the new name is the same as the old name error can still show up, but only if that's what you actually did) :D

[RaafatTurki]

Yeah the issue is fixed, Should I restart testing or are there more commits incoming?

[smjonas]

No more significant commits now, if anything changes now, it will be minor changes like Readme updates :) (The second force-push I created did not change anything relevant.)

[RaafatTurki]

Oh yeah I've just noticed it was a vim.validate() call, Still I did more testing trying to bring out edge cases.

I couldn't rename across files with pyright but that's definitely not an issue with this plugin since I get the same behavior with vim.lsp.buf.rename()

Let me know if you've got any edge cases in mind.

[smjonas]

I know an error will be shown when cancelling the "old" :IncRename command without typing anything and then renaming again, but that is also the case on master so I'll fix that separately. Other than that, I can't really think of any special use cases other than to make sure that we did not break the old command. If you didn't notice anything special I think I can merge, wdyt?

[RaafatTurki]

Just tested :IncRename with sumneko_lua, gopls, clangd and pyright in single and mutiple file renames and it works just fine.

Even pressing esc or C-[ mid-renaming works. Are you sure that's a bug with the plugin itself?

Soon I'll take a closer look at the code and try to find any behavioral differences in both implementations.

Other than that regarding dressing.nvim support I think it's ready.

[smjonas]

Thank you again for your great help with this PR! It really sped things up and reduced the chance of merging an erroneous PR. Let me know if there are other things that can be improved in this plugin, if you notice any in the future!