Olical / nfnl

Enhance your Neovim with Fennel
The Unlicense
235 stars 8 forks source link

Compile-on-save Does not Work on Nightly/0.10 #10

Closed accidentaldevelopment closed 1 year ago

accidentaldevelopment commented 1 year ago

Hello! I was trying this plugin out on nightly (inlay hints!) and automatic compiling is not working.

Minimal Config

local lazypath = vim.fn.stdpath('data') .. '/lazy/lazy.nvim'

if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    'git',
    'clone',
    '--filter=blob:none',
    '--single-branch',
    'https://github.com/folke/lazy.nvim.git',
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

require('lazy').setup({
  spec = {
    'Olical/nfnl',
    ft = 'fennel',
  },
  defaults = {
    lazy = true,
  },
})

I also tried using pack/*/start and pack/*/opt directories, and the results are the same.

I managed to track it down to lazy loading. If lazy loading as above (or with pack/*/opt) then the plugin works after the second fennel file is opened.

Best I can tell, two auto commands are created:

On nightly, the first autocmd is always loaded. But the second is only loaded for every subsequent fennel file.

If I don't lazy load:

require('lazy').setup({
  spec = {
    'Olical/nfnl',
  },
})

Then everything works as advertised on nightly and 0.9.

I'm happy to help with tracking it down further, but not really sure where to start.

Olical commented 1 year ago

Ah that sucks 😞 I guess the autocmd timing has changed or something, I'll try to investigate soon. If you spot a relevant change in the changelogs please do share, it'll help narrow it down.

I'll get the test suite running with 0.10 and also have it open a file and write it as part of an e2e test to catch things like this in the future. I wonder if there's a nightly build I can use in my CI too...

mrjones2014 commented 1 year ago

I wonder if there's a nightly build I can use in my CI too...

You can use setup-vim GitHub Action to install the latest nightly in CI.

accidentaldevelopment commented 1 year ago

I think I tracked it down!

When the plugin is loaded, two things happen:

  1. Plugin is loaded via FileType event matching fennel.
  2. Plugin creates another FileType autocmd to create the BufWritePost autocmd for fennel files.

The issue is that by the time 2 happens, the FileType even has already fired so it won't fire again. Which actually makes me think that the current 0.9 behavior may be a bug in neovim, but that's beside the point.

I can think of two solutions:

I've tested both and they seem to work.

Happy to open a PR if you'd like me to, just let me know which (if either) solution you want to go with. Or could maybe just revert the above commit.

Olical commented 1 year ago

Spent the morning getting an e2e test suite running + added nightly builds https://github.com/Olical/nfnl/actions/runs/5832039232

And the issue doesn't happen! I did discover a relative path bug though which seems to be dependent on local nvim config, this is probably similar.

Olical commented 1 year ago

I've reverted the commit that removed this statement, I don't seem to need it (despite lazy loading, although I'm not on 0.10 yet) and it works in CI without it (although that doesn't lazy load).

So I'll just put that original statement you linked to back in for now, can't hurt and protects against some vague behavior.

accidentaldevelopment commented 1 year ago

I don't seem to need it (despite lazy loading, although I'm not on 0.10 yet) and it works in CI without it (although that doesn't lazy load).

That makes sense. Based on my testing, it only happens with lazy loading on 0.10.

In fact, it was just lazy loading on FileType. If I set it to load on one of the BufRead* events then everything was fine because the FileType still had a chance to fire. I'm even more curious now, so I'll see if I can find where that changed in 0.10.

Either way it seems to be working now! Thanks so much!