OmniSharp / omnisharp-vim

Vim omnicompletion (intellisense) and more for C#
http://www.omnisharp.net
MIT License
1.7k stars 169 forks source link

Cannot rename symbol under cursor by using the command line window. #736

Closed DasOhmoff closed 2 years ago

DasOhmoff commented 2 years ago

Hello. Thank you for your support.

I have an issue that I cannot resolve. When using the command line window and trying to rename the symbol under the cursor, it does not work a lot of times. F.e at the very first time when opening the cs file.

Way to reproduce:

  1. Freshly (re)start vim, and open a cs file
  2. Move cursor over any renameable symbol
  3. Press : and <c-f> to open the command line window
  4. Enter the command call OmniSharp#actions#rename#To("test") into the command line window and execute it

You will see that the output is then No action taken, and that indeed nothing happens then. But when you repeat the same thing twice, then it seems to work sometimes.

This problem occurs in multiple scenarios, and quite frequently to me. Is there a way to fix this?

(I am using neovim, maybe that matters?)

nickspoons commented 2 years ago

What is the significance of the command-line window in this description?

nickspoons commented 2 years ago

Oh. I've found one place where this reliably fails. OmniSharp-vim is reporting an incorrect column position to OmniSharp-roslyn when there are multi-byte characters earlier in the line. Is there any chance that this could be what you're seeing, @DasOhmoff?

nickspoons commented 2 years ago

I'm going to fix the multi-char error, but I think there is still a problem here. However it's on the omnisharp-roslyn side. We sometimes just get an empty result-set from the server.

Other code actions also sometimes don't work the first time, and need to be run again.

DasOhmoff commented 2 years ago

What is the significance of the command-line window in this description?

The significance is that the problem only occurs when using the command line window to execute the command. If one executes the command without the command line window, by simply tying :call OmniSharp#actions#rename#To("test") and pressing enter, then the renaming works perfectly fine. Even for the very first time I use it. Only when I enter the command via the command line window, only then the problem occurs. If I enter the command without opening the command line window, then everything works fine. Steps to reproduce:

  1. Freshly (re)start vim, and open a cs file
  2. Wait until the server is loaded
  3. Move cursor over any rename able symbol
  4. Enter the command :call OmniSharp#actions#rename#To("test") and press enter

In this case, the renaming works correctly.

I'm going to fix the multi-char error, but I think there is still a problem here. However it's on the omnisharp-roslyn side. We sometimes just get an empty result-set from the server.

I am not sure if multi-char error is the problem. But I am certain that the problem cannot be because of omnisharp-roslyn, because the problem only occurs when the command line window is open, in theory this should not effect omnisharp-roslyn in any way (if the input was correctly read from the command line window).

nickspoons commented 2 years ago

Ok. But in your original description you've said to first open the command-line window, then navigate to the symbol you want to rename. Are points 2 and 3 just the wrong way around?

DasOhmoff commented 2 years ago

Ah yes, true, my fault. I changed the description and reordered it correctly now.

nickspoons commented 2 years ago

This doesn't seem to happen any more regularly for me in the command-line window than from normal commands. Here's what seems to happen most often: the first attempt fails (either from the command line or the command-line window), possibly because the server hasn't fully warmed up, and all subsequent renames from the command-line window succeed:

asciicast

DasOhmoff commented 2 years ago

Did you wait for the server to load up? I did not mention that before, but I wait for the Loaded server for *** in ***s message before I enter the commands. So if I wait until the server is loaded and until the message is shown, then the first rename request works on my machine immediately. But when I do the exact same thing, and use the command line window, then it does not work.

Here is a demonstration:

https://user-images.githubusercontent.com/16063186/139580472-b5d88090-5897-4f8d-afb4-0ad50810068b.mp4

This is the whole config that I used in this video.

set runtimepath^=~/VimFiles
let &packpath = &runtimepath

call plug#begin("~/VimFiles/plugged")
Plug 'OmniSharp/omnisharp-vim'
call plug#end()
nickspoons commented 2 years ago

Ohhhhhhh I see what's happening.

The rename request is happening correctly. The server receives the async response and begins preparing the response. In the meantime, focus is returned to the main buffer, which triggers a BufEnter auto-command, which a /highlight requests to occur. This request sends the content of the buffer, which has not yet been updated. This content updates the server version of the buffer, overriding the rename. So when the response from the rename request arrives, it contains no changes, as the rename changes have been overridden.

The underlying issue is that we send the current state of the buffer along with basically all requests. It would be good to stop doing this and only update the state of the buffer when there have been changes, but this is a relatively big change and my tentative attempts so far have been buggy, it's hard to get right.

Requires some work and thought.

DasOhmoff commented 2 years ago

Ah I see. I am pleased to hear that you have found the issue. Do you think you will be able to fix it this week?

nickspoons commented 2 years ago

Do you think you will be able to fix it this week?

I doubt it, I don't have very much free time at the moment. Hmm. Just disabling the buffer update with the /highlight request might be a valid solution though...

Do you think you will be able to fix it this week?

Maybe.

nickspoons commented 2 years ago

@DasOhmoff that one-line change appears to fix the issue for me, please confirm!

DasOhmoff commented 2 years ago

Yes! It works! Thank you.

DasOhmoff commented 2 years ago

Hey, I just realized something. I think this maybe caused another issue. Highlighting seems to break in certain circumstances. F.e when entering text with the o command and undoing it afterwards. Steps to reproduce:

  1. Set let OmniSharp_highlight_types = 2 (maybe this is not necessary)
  2. Open a cs file and place cursor at a line
  3. Press o and start entering some random text. F.e ------------------- or so (this should get highlighted as an c# syntax error)
  4. Exit the insert mode and wait until the error highlighting appears
  5. Press u to undo the insertion
  6. Now there should be wrong highlighting below the line that you undid. Most likely the highlighting does not get updated.

I think this issue is caused by the last commit.

nickspoons commented 2 years ago

Thanks for reporting that @DasOhmoff. I had noticed this too, and started trying to clean up my old buffer-update refactor that I mentioned earlier. It's not quite finished but I think it fixes this situation, are you able to test it out and see if it improves things for you? PR #738

DasOhmoff commented 2 years ago

I pulled your changes in #738 and tested them out. It seems that they are working, when changing the buffer contents the highlighting seems to be correct. I could not find any issues with the highlighting.

But I noticed something else with these changes, maybe you know this already, but in case you don't: sometimes errors or warnings are shown when there actually are none. Example:

  1. Set let OmniSharp_highlight_types = 2 (maybe this is not necessary)
  2. Open a cs file and place your cursor at a new line within a method
  3. Enter int test = 3; (this should be a valid c# statement without any errors)
  4. Exit the insert mode and wait until the error highlighting appears
  5. Now you should be able to see the following error: ; expected

Thank you for your efforts.

nickspoons commented 2 years ago

Replied in #738