-
Notifications
You must be signed in to change notification settings - Fork 2k
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
base: master
Are you sure you want to change the base?
Conversation
@@ -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' } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BufNewFile
run before FileType
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
0aa499d
to
2ccafd1
Compare
I can't think of a better solution than 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 I looked at the Nvim source to try to confirm that |
This fixes #2711 by hooking
try_add()
andtry_add_wrapper()
toBufNewFile
in addition to the existing hook onBufReadPost
.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.