ray-x / go.nvim

G'day Nvimer, Joyful Gopher: Discover the Feature-Rich Go Plugin for Neovim
MIT License
1.91k stars 119 forks source link

Configuration option to tell go.nvim to never load guihua #393

Closed bruxisma closed 8 months ago

bruxisma commented 8 months ago

I was able to track this down behavior found in utils.load_plugin, where if packer or lazy.nvim are detected, go.nvim will attempt to load guihua. However, unlike the pcall attempts for checking for lazy or packer, the call to load guihua is done so unconditionally:

      require('lazy').load({ plugins = name })

This results in a vim.notify error every time any function might load guihua. Unfortunately, while it might be easy to say "just use it then", this makes it impossible for me to use the current vim.ui.input plugin that I actually use, and prefer to use, over guihua. This is because projects where I write go are usually multi-lingual and so once guihua is loaded my input and select settings are changed until I reset my neovim instance.

Given that the function in question can return nil, I would very much prefer there to be a "bring your own vim.ui.input/vim.ui.select plugin" setting if possible to just unconditionally return nil if found.

ray-x commented 8 months ago

Not sure if there a specific module/function breaks? As input will be override only during the time you using it and will be reset back later on. Here is code from go rename which uses input:

  local guihua = utils.load_plugin("guihua.lua", "guihua.floating")
  local input = vim.ui.input

  if guihua then
    vim.ui.input = require('guihua.input').input
  end
  vim.lsp.buf.rename()
  return vim.defer_fn(function()
    vim.ui.input = input  -- after rename it will reset 
  end, 1000)
bruxisma commented 8 months ago

It seems to set it across all buffers once it's been enabled, and if I swap to a different buffer mid-rename it'll continue to use it and that's happened on occasion, but only when using gui clients where buffers don't steal the entire context input.

That said, I should point out that in the README, it does say guihua is an optional dependency for floating arguments, but the way utils.load_plugin executes above, it will always results in an error under lazy.nvim, and if I don't have a custom notify that pops up under :messages. tbqh I wasn't sure if I should just request a way to disable it, or ask that how all plugins are loaded with lazy.nvim change (edit: so that it is protected by a pcall). I figured asking for a configuration option would be the least intrusive change. 😅

ray-x commented 8 months ago

Use GoRename for example, if guihua is installed, it use guihua. After rename finished, the vim.ui.input will reset back. It does not affect other components.

guihua is an optional package and if it fails to load it will fallback to default non- UI output instead of preventing you from using the plugin.

Also, you can adjust the vim.notify to be less noisy and only popup when warn/error. Alternatively, you can override go.utils.info to a print function or empty function if you think it is necessary

bruxisma commented 8 months ago

Unfortunately, lazy.nvim always errors when guihua is not installed, hence why I'm asking, if this configuration could be added to just skip all of this together. I would need to turn off all of notify itself, or modify lazy.nvim directly.

image

This error occurs everytime I use anything that might load guihua in go.nvim, the primary one being GoRename

If there was a configuration option to either skip, or a change made to util.load_plugins to pcall the lazy.load call, I wouldn't be as worried about the situation.

ray-x commented 8 months ago

I have updated a change so it will be less noisy.

bruxisma commented 8 months ago

Hi, I was looking at the commit and it looks like it's currently in the fixtures branch. 😅 I appreciate the work though. Looking forward to the commit making it into the main branch 🙂

ray-x commented 8 months ago

Thanks for the heads up. The branch was merged

bruxisma commented 8 months ago

Thank you! I'll close this now 🙂