Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
6 tasks done
Svalorzen opened this issue Feb 26, 2024 · 4 comments
Open
6 tasks done

Comments

@Svalorzen
Copy link

Svalorzen commented Feb 26, 2024

Issue Prelude

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

  • I have read and understood YCM's [CONTRIBUTING][cont] document.
  • I have read and understood YCM's [CODE_OF_CONDUCT][code] document.
  • I have read and understood YCM's [README][readme], especially the
    [Frequently Asked Questions][faq] section.
  • I have searched YCM's issue tracker to find issues similar to the one I'm
    about to report and couldn't find an answer to my problem.
  • I understand this is an open-source project staffed by volunteers and
    that any help I receive is a selfless, heartfelt gift of their free time. I
    know I am not entitled to anything and will be polite and courteous.
  • I understand my issue may be closed if it becomes obvious I didn't
    actually perform all of these steps.

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
Copy link
Collaborator

bstaletic commented Feb 27, 2024

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
Copy link
Author

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:

  • :tab split which creates a new tab, duplicating the currently opened (dirty) buffer, AND also switches to it automatically. YCM is not invoked yet.
  • :YcmCompleter GoToDefinition is then called from the new tab (which is simply the current buffer and has no other influence on the command aside this). This results in a split as the current buffer (in the newly opened tab) is dirty. The previous tab with the exact same open dirty buffer still exists.

I hope this is a bit more clear.

@bstaletic
Copy link
Collaborator

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants