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

Enhancement: expose ID of window created by vim.ui.input #58

Closed
smjonas opened this issue Aug 27, 2022 · 5 comments
Closed

Enhancement: expose ID of window created by vim.ui.input #58

smjonas opened this issue Aug 27, 2022 · 5 comments

Comments

@smjonas
Copy link

smjonas commented Aug 27, 2022

Hi,
I'm currently trying to add proper support for dressing.nvim in my plugin inc-rename.nvim (the current implementation has issues, see smjonas/inc-rename.nvim#17). Basically, I need to control the window created by dressing.nvim from the command line and change the input text while the user is typing. For this to work, I need to get the ID of the window created by vim.ui.input.

At the moment, I am doing the following to find the ID:

-- Open the input window and find the buffer and window IDs
vim.ui.input({}, function() end)
vim.schedule(function()
  for _, win_id in ipairs(vim.api.nvim_list_wins()) do
    local bufnr = vim.api.nvim_win_get_buf(win_id)
    if vim.bo[bufnr].filetype == "DressingInput" then
      -- found the window ID
    end
  end
end)

This almost works but using vim.schedule is problematic as that doesn't work well with Nvim's command preview callback. Omitting vim.schedule won't work because you use schedule_wrap in input.lua here (so I need to schedule my code to run after yours):

__call = vim.schedule_wrap(function(_, opts, on_confirm)

Therefore, I am proposing the following enhancements:

  • add support for a boolean parameter no_schedule / synchronous / sync passed to the opts key in vim.ui.input which won't schedule wrap the function if set to true (let me know which name you'd prefer)
  • if no_schedule / synchronous / sync is set, return the window ID from vim.ui.input

Please let me know if you have any another ideas / suggestions! I can send a PR if you want :)

@stevearc
Copy link
Owner

The original reason why vim.ui.input was wrapped was to work around an issue during neovim startup (see #15). I've updated the wrapper to only use schedule_wrap during startup, and to execute synchronously otherwise. I believe that inc-rename will only ever be called after startup, so this should make all vim.ui.input calls from inc-rename synchronous. Does that work for you?

I'm a little more hesitant to return the window ID from the call because this starts to change the default built-in API. There is some precedent for this, as I decided to add an API extension in e607dd9, but I want to be extra cautious about these sorts of things and preferably do it in a way that is guaranteed to never conflict with the built-in API.

@smjonas
Copy link
Author

smjonas commented Aug 28, 2022

Thanks a lot, this should resolve my main issue. I'm fine with leaving it like this if you don't want to change the API since there exists a workaround to get the ID. Still, I'm curious what exactly the API contract is here / why is returning something instead of nil problematic?

@smjonas smjonas closed this as completed Aug 28, 2022
@stevearc
Copy link
Owner

Two reasons:

  1. It's possible that neovim will refactor vim.ui.input to return some value in the future, at which point it will clash. Not incredibly likely, but not beyond the realm of possibility.
  2. It introduces a non-standard API that users or plugin authors could start to depend on. They might start to expect that if vim.ui.input returns nil, it doesn't open a floating window. That in turn might cause bugs or unexpected behavior if someone uses a vim.ui.input implementation that isn't dressing.nvim.

These are small potential issues and I could be convinced otherwise, but in general I think that maintaining exact API compatibility is valuable and should not be broken lightly.

@stevearc
Copy link
Owner

Note that I had to revert this change to vim.ui.select because it turns out we need the schedule_wrap for other reasons when opening the select windows. vim.ui.input is unchanged, so hopefully this will not affect you.

@smjonas
Copy link
Author

smjonas commented Aug 31, 2022

Thanks for letting me know! No, as it stands now this won't affect me :)

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