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

Bug: opening LSP definition in a vsplit in an unsaved buffer causes annoying message to popup #1677

Closed
4 of 6 tasks
iniw opened this issue Jan 6, 2025 · 21 comments
Closed
4 of 6 tasks
Labels
bug Something isn't working

Comments

@iniw
Copy link

iniw commented Jan 6, 2025

RTFM Checklist

  • I have searched exisiting issues / discussions
  • I have read the Wiki including the Advanced section
  • I have read man fzf / I am well versed in shell fzf

Operating system

NixOS

Shell

zsh

Neovim version (nvim --version)

NVIM v0.10.2

Fzf version (fzf --version)

0.56.3 (v0.56.3)

Output of :lua print(os.getenv('FZF_DEFAULT_OPTS'))

nil

Is the problem reproducible with mini.sh?

  • My issue is reproducible with mini.sh
  • My issue IS NOT reproducible with mini.sh
  • I have not tested with mini.sh (not relevant, requires LSP, Windows, etc)

Fzf-lua configuration

lazy.nvim config:

return {
  {
    "ibhagwan/fzf-lua",
    keys = {
      {
        "gad",
        function()
          local fzf_lua = require("fzf-lua")
          fzf_lua.lsp_definitions({
            sync = true,
            ignore_current_line = true,
            jump_to_single_result = true,
            jump_to_single_result_action = fzf_lua.actions.file_vsplit,
            actions = {
              ["enter"] = fzf_lua.actions.file_vsplit,
            },
          })
        end,
        desc = "Goto Definition in a New Window",
      },
    },
  },
}

Describe the bug / steps to reproduce

When I use gad with that config it opens the definition of the current symbol in a vertical split. The problem is that when I'm working on a modified and not-yet-saved buffer a message pops up complaining that I have to save the buffer before continuing. This is very disruptive.

Cursor hovering over symbol, before I press gad:
image

gad pressed:
image

C pressed, to "Cancel" the popup:
image

I am not exactly certain that this is a fzf-lua issue, but the whole "go to definition and split the buffer vertically" logic is done by fzf-lua, so it sounds like the right place to report it.

@iniw iniw added the bug Something isn't working label Jan 6, 2025
@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

I am not exactly certain that this is a fzf-lua issue, but the whole "go to definition and split the buffer vertically" logic is done by fzf-lua, so it sounds like the right place to report it.

Correct fzf-lua is to blame here but it shouldn’t popup in a vsplit (only if it replaces the current buffer).

I’m assuming you’re not using :set nohidden - because that would make it go away too.

@iniw
Copy link
Author

iniw commented Jan 6, 2025

I’m assuming you’re not using :set nohidden - because that would make it go away too.

It happens even with nohidden/hidden = 0

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

I’m confused how can this popup with nohidden, I’ll have to check this later when not AFK.

The condition below can’t happen with hidden, maybe there’s another location I call save dialog but I can’t recall ATM

local will_replace_curbuf = vimcmd == "e" or vimcmd == "b"
if will_replace_curbuf
and not vim.o.hidden
and not vim.o.autowriteall
and utils.buffer_is_dirty(nil, false, true) then
-- when `:set nohidden`, confirm with the user when trying to switch
-- from a dirty buffer, abort if declined, save buffer if requested
if utils.save_dialog(nil) then
vimcmd = vimcmd .. "!"
else
return
end

@iniw
Copy link
Author

iniw commented Jan 6, 2025

Screencast.From.2025-01-06.20-03-14.webm

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

I believed you without the video, I’m just befuddled, until I can test this myself :)

@iniw
Copy link
Author

iniw commented Jan 6, 2025

I'm investigating the issue. If I find anything I'll write a PR.

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

I'm investigating the issue. If I find anything I'll write a PR.

Ty @iniw, AFAIK the location I linked is the only place save dialog is called in my code.

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

Btw what’s the value or your :set confirm?

@iniw
Copy link
Author

iniw commented Jan 6, 2025

Ty @iniw, AFAIK the location I linked is the only place save dialog is called in my code.

Yeah, it is, and that branch is not being taken. Here's a log of all relevant variables in the if condition: will_replace = false, hidden = true, writeall = false, dirty = true.

Looks like the problem is coming from somewhere else.

Btw what’s the value or your :set confirm?

That's a command AFAIK, wdym what value is set?

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

That's a command AFAIK, wdym what value is set?

Yes I meant what’s the output of :set confirm? or := vim.o.confirm?

@iniw
Copy link
Author

iniw commented Jan 6, 2025

Yes I meant what’s the output of :set confirm? or := vim.o.confirm?

Just true

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

Yes I meant what’s the output of :set confirm? or := vim.o.confirm?

Just true

This might be the issue, see :help 'confirm'.

Can you run :set noconfirm and try?

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

'confirm' 'cf'		boolean	(default off)
			global
	When 'confirm' is on, certain operations that would normally
	fail because of unsaved changes to a buffer, e.g. ":q" and ":e",
	instead raise a dialog asking if you wish to save the current
	file(s).  You can still use a ! to unconditionally |abandon| a buffer.
	If 'confirm' is off you can still activate confirmation for one
	command only (this is most useful in mappings) with the |:confirm|
	command.
	Also see the |confirm()| function and the 'v' flag in 'guioptions'.

@iniw
Copy link
Author

iniw commented Jan 6, 2025

Can you run :set noconfirm and try?

That prevents the popup from appearing but now I get the following warning: [Fzf-lua] ':vsplit | e' failed: vim/_editor.lua:0: nvim_exec2(): Vim(edit):E37: No write since last change (add ! to override)

And also, doesn't setting noconfirm mean that I lose all of the confirm dialogs, including the ones not originating from fzf-lua? Those are useful, I'd like to keep them...

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 6, 2025

And also, doesn't setting noconfirm mean that I lose all of the confirm dialogs, including the ones not originating from fzf-lua? Those are useful, I'd like to keep them...

At least now we know what’s triggering the popup so I can solve it, I’m still not sure why this happens but I have a guess.

can you add _uri = false to your lsp definitions options (from the OP)?

@iniw
Copy link
Author

iniw commented Jan 6, 2025

can you add _uri = false to your lsp definitions options (from the OP)?

That does fix it! Thank you.

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 7, 2025

can you add _uri = false to your lsp definitions options (from the OP)?

That does fix it! Thank you.

Fantastic, now I know exactly what’s happening, using LSP pickers fzf-lua converts entries to URI so it can call specialty entries like jdt://… and also be added to the neovim tagstack.

We just turned the URI conversion off but you’re not missing out on much.

I’ll look into fixing this later today.

@iniw
Copy link
Author

iniw commented Jan 7, 2025

We just turned the URI conversion off but you’re not missing out on much.

Yeah I don't think I've ever used that feature.

I’ll look into fixing this later today.

Fantastic, I'll add the _uri = false workaround to my config in the meantime.

Thanks for the excellent support today. The two issues I reported were super annoying, makes me so glad to have them fixed! Thanks a lot, again.

iniw added a commit to iniw/system that referenced this issue Jan 7, 2025
@ibhagwan
Copy link
Owner

ibhagwan commented Jan 7, 2025

We just turned the URI conversion off but you’re not missing out on much.

Yeah I don't think I've ever used that feature.

I’ll look into fixing this later today.

Fantastic, I'll add the _uri = false workaround to my config in the meantime.

Thanks for the excellent support today. The two issues I reported were super annoying, makes me so glad to have them fixed! Thanks a lot, again.

Ty for being so responsive, the only way we get to perfection :)

iniw added a commit to iniw/system that referenced this issue Jan 7, 2025
@ibhagwan
Copy link
Owner

ibhagwan commented Jan 7, 2025

12b4bc6

@iniw, should work as expected now, lmk if that's not the case.

@ibhagwan
Copy link
Owner

ibhagwan commented Jan 7, 2025

87eff1a

Found some more edge cases regarding same buffer splits and hidden setting, this is a better and more complete solution not limited to LSP URIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants