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

Consider attaching server when editing new file, not just existing files, when autostart = false #2712

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Diomendius
Copy link

This fixes #2711 by hooking try_add() and try_add_wrapper() to BufNewFile in addition to the existing hook on BufReadPost.

I did consider including a test with this PR, but there doesn't seem to be any existing test infrastructure for tests involving actually running a language server (or a mock of one), and the upfront cost of adding this would be much greater than that of this two-line commit.

@@ -100,7 +100,7 @@ function configs.__newindex(t, config_name, config_def)

if config.autostart == true then
local event_conf = config.filetypes and { event = 'FileType', pattern = config.filetypes }
or { event = 'BufReadPost' }
or { event = { 'BufReadPost', 'BufNewFile' } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BufNewFile run before FileType

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

au FileType * echo 'FileType ' . &ft
au BufNewFile * echo 'BufNewFile ' . &ft
au BufReadPost * echo 'BufReadPost ' . &ft

Using the above for testing, both BufNewFile and BufReadPost run before FileType. Weirdly, when I run these commands with my usual config loaded, FileType actually executes first. I have no idea what part of my config changes the order.

If there's any issue with this callback triggering before FileType, it affects BufReadPost as well. I'm not sure if it matters here, since BufReadPost and BufNewFile are only used if config.filetypes is unset. Does it make sense for a server to be configured without config.filetypes if it matters what filetype is when try_add() runs?

At line 139 it's definitely important that try_add_wrapper() runs after filetype is set, however.

vim.schedule() seems to be enough to defer a function until after FileType, but this seems like a hacky solution to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the following in test.lua:

vim.api.nvim_create_autocmd({ "BufReadPost", "FileType", "BufNewFile" }, {
    callback = function(e)
        print(vim.o.ft, vim.inspect(e))
    end,
})

If I run XDG_DATA_HOME=xdg_data XDG_STATE_HOME=xdg_state nvim -u test.lua, BufReadPost/BufNewFile triggers first.
If I run XDG_DATA_HOME=xdg_data XDG_STATE_HOME=xdg_state nvim -u NORC +source\ test.lua, FileType triggers first.
If I wrap the entire contents of test.lua in vim.schedule(), FileType always triggers first regardless of whether I :source test.lua or load it as the rc file with -u.

BufReadPost and BufNewFile autocommands only seem to trigger earlier than FileType if they were set up sufficiently early during startup. This seems weird to me, but I can reproduce it consistently. I don't know what the intended execution order of these events is, or if there is even meant to be a guaranteed order at all.

BufReadPost does not trigger when editing a nonexistent file, so
language servers would not automatically attach when editing a new file
if autostart was disabled or if the server had no associated filetypes.
This fixes that by adding BufNewFile to autocommands hooked to
BufReadPost.
Sometimes, BufNewFile triggers before 'filetype' is set. Using
vim.schedule() should ensure filetype detection runs before the
callback.
@Diomendius
Copy link
Author

I can't think of a better solution than vim.schedule(), but it seems to work fine in testing and in normal use.

It'd be nice to have an explicit guarantee that the callback will run only after filetype detection. The other idea I had was to create a one-shot FileType autocommand when triggering on BufNewFile, but this just seems generally worse and would break if FileType fires before BufNewFile, which does happen sometimes, seemingly depending on user configuration.

I looked at the Nvim source to try to confirm that vim.schedule() callbacks couldn't fire between BufNewFile and FileType, but I couldn't make much sense of it. It should be enough to confirm that every code path that can trigger BufNewFile also triggers (or has already triggered) filetype detection before the event loop runs. Intuitively, I would expect this to be the case, and it appears to be in practice.

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

Successfully merging this pull request may close these issues.

Server does not automatically attach when editing nonexistent file in workspace when autostart = false
2 participants