stevearc / dressing.nvim

Neovim plugin to improve the default vim.ui interfaces
MIT License
1.73k stars 32 forks source link

Rename is not working as expected #5

Closed augustocdias closed 2 years ago

augustocdias commented 2 years ago

I have a keymap set to <cmd>lua vim.lsp.buf.rename()<CR>. When I invoke it dressing shows the float input as expected. When I hit enter to make the rename it changes only the position where I invoked instead of all occurrences.

When I removed dressing and it went back to the traditional input method it worked as expected.

I don't know if it happens with all LSP servers, but I tested with rust.

stevearc commented 2 years ago

That is a pretty crazy bug. I tested rust-analyzer and it seems to be working for me. If you were seeing no changes I'd say it's a bug with the input modal, but given that something is changing it seems like the UI is passing the value on and the LSP is choking. The weird part then is I don't know why it would be behaving differently with one UI vs the other.

You could try putting a print statement in the on_confirm callback to verify that it's getting passed the correct value in either case. This doesn't even require building from source since it's in the lua files; you just have to find the runtime/ dir for your neovim install.

augustocdias commented 2 years ago

It is printing the new name as input... This is really weird... I setup a sample simple project and it worked as expected.

But my real complex one is having this issue...

Any idea what can I look into to find the root cause of it?

augustocdias commented 2 years ago

I've added a print on the on_confirm from dressing and found something strange

    print(text)
    print(context.on_confirm)

Is it supposed to be called twice and passing nil in the second time?

database1
function: 0x0107949510
nil
nil
stevearc commented 2 years ago

The double call is expected. The first call is the "confirm" message we get from <CR>, and the second one is the "cancel" message from the autocmd that triggers when closing the window. The second one will be ignored because context.on_confirm is nil.

I did get another idea while looking through the rename source code. Using input() doesn't move your cursor around. Using a floating window does. The rename command is going to be constructing arguments based on your cursor position, so that could affect it.

Where to go from here:

  1. I pushed up a commit that might change things for you. Theoretically it shouldn't affect any functionality, but it removes a hack related to cursor movement so who knows maybe it has an impact
  2. If that doesn't fix it, could you print out the util.make_position_params()? I suspect those values are different.
  3. If the rust project is open source, could you point me to it? If not, could you give me a bit more info about the symbol you're trying to rename (is it a variable? function? struct? Is your cursor at the beginning/middle/end? Are you renaming from the declaration or a reference?)
augustocdias commented 2 years ago

That solved the issue. :)

augustocdias commented 2 years ago

I'm having the issue again :(

I have no idea caused it to start again

the print you asked

table: 0x010dd3ef98
position table: 0x010ded7f50
character 24
line 19
textDocument table: 0x010de6a808
uri file:///Users/augusto/projects/nofitier/src/tests/templates.rs

and in one project that it works

table: 0x010836fc88
textDocument table: 0x01083704a8
uri file:///Users/augusto/projects/neovide/src/main.rs
position table: 0x0108370638
line 144
character 27

Unfortunately the project is not open source, so I cannot share... I'll see if I can make it happen in one that is open source.

The print statements I added to the runtime

        print(util.make_position_params())
        for k, v in pairs(util.make_position_params()) do
            print('\t', k, v)
            for k1, v1 in pairs(v) do
                print('\t', k1, v1)
            end
        end

What I noticed it is not in the whole project that the issue happens. In some parts it works as expected

augustocdias commented 2 years ago

I think I found the pattern. It seems it doesn't work in tests

In my setup, I tried to rename the req variable in this file: https://github.com/actix/actix-web/blob/master/src/test/test_request.rs#L428

and it changed only the declaration and not the reference(s).

augustocdias commented 2 years ago

An it is not an issue with dressing as it turns out... With vanilla input happens too.