ycm-core / YouCompleteMe

A code-completion engine for Vim
http://ycm-core.github.io/YouCompleteMe/
GNU General Public License v3.0
25.36k stars 2.8k forks source link

GoToDefinition splits instead of replacing buffer if modified but opened elsewhere #4223

Open Svalorzen opened 4 months ago

Svalorzen commented 4 months ago

Issue Prelude

Please complete these steps and check these boxes (by putting an x inside the brackets) before filing your issue:

Thank you for adhering to this process! It ensures your issue is resolved quickly and that neither your nor our time is needlessly wasted.

Issue Details

I like to both manually use :YcmCompleter GoTo commands, while also keeping a keybind to use them while directly opening the resulting file/line in a new tab. As mentioned in #1146, this can be achieved by setting g:ycm_goto_buffer_command to same-buffer, and creating the following keybind:

nnoremap f :tab split \| YcmCompleter GoToDefinition<CR>

Now something I noticed is that from time to time this would still result in a split. After reading again the manpage for g:ycm_goto_buffer_command, I now realize the reason for this:

If this option is set to the "'same-buffer'" but current buffer can not be
switched (when buffer is modified and 'nohidden' option is set), then result
will be opened in a split.

Indeed, if my starting buffer is modified, my custom map will open a new tab, then call YcmCompleter GoToDefinition, see that the buffer is modified, and create new split.

However, this is completely unnecessary, as there already exists an opened window (the one I left in the original tab) containing the same modified buffer, so there is no chance of the changes being lost if jumping within the same buffer. For this reason, I'd like to suggest that if it can be proven by YCM that the current buffer, although modified, is already opened elsewhere, that the same-buffer policy can be fully respected regardless of the state of the buffer in the current window.

bstaletic commented 4 months ago

The suggested behaviour feels very wrong to me. You told YCM to open the definition in the same buffer youbare in and expect YCM to switch a tab and maybe still end up with a tabpage with a bunch of splits?

I am also of the opinion thatvnohidden is very much unusable in general, but the other maintainer of YCM disagrees.

Personal preferences aside, nohidden also in5roduces a pe4formance hit. In general, through higher amount of IO operations. In YCM specifically, with clangd, through constantly invalidating the preamble.

Svalorzen commented 4 months ago

The suggested behaviour feels very wrong to me. You told YCM to open the definition in the same buffer youbare in and expect YCM to switch a tab and maybe still end up with a tabpage with a bunch of splits?

No, sorry if I was not clear. The issue is that calling :YcmCompleter GoToDefinition in a dirty buffer with same-buffer set will always split, even if the buffer is opened in another window and/or in another tab. In this case there should be no problem in replacing the buffer with the jump as it is not possible to lose the changes.

My personal example where I opened a tab was just my personal use-case for this. Just to clarify further, in my case, the sequence of actions performed is:

I hope this is a bit more clear.

bstaletic commented 4 months ago

Right, that makes more sense, but I'd have to check if vim itself agrees and if there's a noticable performance hit for the usual use case.

I am on a vacation currently, so next week, probably.

Svalorzen commented 4 months ago

Fair enough, it's definitely not urgent. Thanks a lot for looking into it!