wbthomason / packer.nvim

A use-package inspired plugin manager for Neovim. Uses native packages, supports Luarocks dependencies, written in Lua, allows for expressive config
MIT License
7.89k stars 264 forks source link

filetype option (ft = ...) loads files within ftplugin/ folders twice #648

Open ecogest opened 3 years ago

ecogest commented 3 years ago

Features: +acl +iconv +tui


- `git --version`: `git version 2.30.1 (Apple Git-130)`
- Operating system/version: macOS Big Sur, version 11.6
- Terminal name/version: alacritty, version 0.9.0
- Packer commit : `257d6d3`

### Steps to reproduce
- Add a package conditionally to a given filetype. Ex: `use {'alepez/vim-gtest', ft = 'cpp'}`

- Create a file for the given filetype in `~/.config/nvim/ftplugin` (or after/ftplugin), and count how many times it is loaded. Ex: `~/.config/nvim/ftplugin/after/cpp.vim` with some code like :
```vim
if !exists("b:count")
    let b:count = 1
else
    let b:count += 1
endif
" remark: this should not be placed after a guard like `if exists('b:plugin_loaded') | finish | else | let b:plugin_loaded = 1 | endif`

Counted twice (b:count equals 2). Hence the file in ftplugin has been loaded twice. Note: when I run :scriptnames, I don't see it loaded twice though...

Expected behaviour

The file should be loaded once. For example, if you use use {'alepez/vim-gtest'} (without ft = 'cpp'), it is working correctly (~/.config/nvim/ftplugin/after/cpp.vim loaded only once, b:count equals 1).

Due to this issue, the options I set in after/ftplugin, get overwritten for some filetypes (I use a guard so that it is not loaded twice). I don't really understand how, but some other ftplugin files, which should be loaded before, manage then to overwrite my own in the after folder...

packer files

packer log file Post the contents of ~/.cache/nvim/packer.nvim.log here : empty

thanks anyway

Thank you so much for packer, I find it awesome 😉 !

wbthomason commented 3 years ago

Thanks for your report! Just to verify, your version of Neovim is after this PR, right? https://github.com/neovim/neovim/pull/15295

Assuming so, this is probably a bug with https://github.com/wbthomason/packer.nvim/blob/257d6d30e4bd4ab8f5d2a40d73a9f1a4a65779fc/lua/packer/load.lua#L110-L114 or https://github.com/wbthomason/packer.nvim/blob/257d6d30e4bd4ab8f5d2a40d73a9f1a4a65779fc/lua/packer/load.lua#L41-L45

I suspect the former snippet more because you say that you can reproduce this issue with or without files in after/. To eliminate the second candidate, would you mind sharing your compiled file? (the files which that block sources are listed in the compiled output)

The last time we changed something in the first candidate seems to have been #533. I am surprised that you're not seeing the double-load in scriptnames, too.

ecogest commented 3 years ago

Thanks for your reply ! I reproduced the issue with the minimum number of plugins :

packer compiled file ```lua -- Automatically generated packer.nvim plugin loader code if vim.api.nvim_call_function('has', {'nvim-0.5'}) ~= 1 then vim.api.nvim_command('echohl WarningMsg | echom "Invalid Neovim version for packer.nvim! | echohl None"') return end vim.api.nvim_command('packadd packer.nvim') local no_errors, error_msg = pcall(function() local time local profile_info local should_profile = false if should_profile then local hrtime = vim.loop.hrtime profile_info = {} time = function(chunk, start) if start then profile_info[chunk] = hrtime() else profile_info[chunk] = (hrtime() - profile_info[chunk]) / 1e6 end end else time = function(chunk, start) end end local function save_profiles(threshold) local sorted_times = {} for chunk_name, time_taken in pairs(profile_info) do sorted_times[#sorted_times + 1] = {chunk_name, time_taken} end table.sort(sorted_times, function(a, b) return a[2] > b[2] end) local results = {} for i, elem in ipairs(sorted_times) do if not threshold or threshold and elem[2] > threshold then results[i] = elem[1] .. ' took ' .. elem[2] .. 'ms' end end _G._packer = _G._packer or {} _G._packer.profile_output = results end time([[Luarocks path setup]], true) local package_path_str = "/Users/matthieu/.cache/nvim/packer_hererocks/2.1.0-beta3/share/lua/5.1/?.lua;/Users/matthieu/.cache/nvim/packer_hererocks/2.1.0-beta3/share/lua/5.1/?/init.lua;/Users/matthieu/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/luarocks/rocks-5.1/?.lua;/Users/matthieu/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/luarocks/rocks-5.1/?/init.lua" local install_cpath_pattern = "/Users/matthieu/.cache/nvim/packer_hererocks/2.1.0-beta3/lib/lua/5.1/?.so" if not string.find(package.path, package_path_str, 1, true) then package.path = package.path .. ';' .. package_path_str end if not string.find(package.cpath, install_cpath_pattern, 1, true) then package.cpath = package.cpath .. ';' .. install_cpath_pattern end time([[Luarocks path setup]], false) time([[try_loadstring definition]], true) local function try_loadstring(s, component, name) local success, result = pcall(loadstring(s)) if not success then vim.schedule(function() vim.api.nvim_notify('packer.nvim: Error running ' .. component .. ' for ' .. name .. ': ' .. result, vim.log.levels.ERROR, {}) end) end return result end time([[try_loadstring definition]], false) time([[Defining packer_plugins]], true) _G.packer_plugins = { ["packer.nvim"] = { loaded = true, path = "/Users/matthieu/.local/share/nvim/site/pack/packer/start/packer.nvim" }, ["tokyonight.nvim"] = { loaded = false, needs_bufread = false, path = "/Users/matthieu/.local/share/nvim/site/pack/packer/opt/tokyonight.nvim" }, ["vim-javascript"] = { loaded = false, needs_bufread = true, path = "/Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript" } } time([[Defining packer_plugins]], false) vim.cmd [[augroup packer_load_aucmds]] vim.cmd [[au!]] -- Filetype lazy-loads time([[Defining lazy-load filetype autocommands]], true) vim.cmd [[au FileType javascript ++once lua require("packer.load")({'vim-javascript'}, { ft = "javascript" }, _G.packer_plugins)]] time([[Defining lazy-load filetype autocommands]], false) vim.cmd("augroup END") vim.cmd [[augroup filetypedetect]] time([[Sourcing ftdetect script at: /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/flow.vim]], true) vim.cmd [[source /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/flow.vim]] time([[Sourcing ftdetect script at: /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/flow.vim]], false) time([[Sourcing ftdetect script at: /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/javascript.vim]], true) vim.cmd [[source /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/javascript.vim]] time([[Sourcing ftdetect script at: /Users/matthieu/.local/share/nvim/site/pack/packer/opt/vim-javascript/ftdetect/javascript.vim]], false) vim.cmd("augroup END") if should_profile then save_profiles() end end) if not no_errors then vim.api.nvim_command('echohl ErrorMsg | echom "Error in packer_compiled: '..error_msg..'" | echom "Please check your config for correctness" | echohl None') end ```
plugins.lua file ```lua return require('packer').startup(function() -- Packer can manage itself use 'wbthomason/packer.nvim' use {'folke/tokyonight.nvim', opt = true } use {'pangloss/vim-javascript', ft = {'javascript'} } end) ```
after/ftplugin/javascript.vim ```vim if !exists("b:count") let b:count = 1 else let b:count += 1 endif ```

Bottom line : vim test.js, echo b:count -> 2

On the contrary if I do not use ft = {'javascript'} (use {'pangloss/vim-javascript'}): vim test.js, echo b:count -> 1

ecogest commented 3 years ago

And for the first question yes, I am using the HEAD version, I updated it before posting this issue. (Built from source on 2021-10-12 at 14:05:11).

wbthomason commented 3 years ago

Thanks! This is thoroughly bizarre. I actually get b:count = 3 when I try to reproduce this, which is even weirder. It looks like I get b:count = 2 without any ft loading, so I'm still seeing the same issue as you, but with a confounding factor of some other source executing my after/ftplugin files.

wbthomason commented 3 years ago

Ok, I might know what's going on, if not how to fix it:

Neovim itself sources filetype and after/filetype files when you load a file with a detectable filetype, as expected. packer additionally triggers FileType for the relevant filetype when loading a plugin by ft because that plugin may have its own ftplugin or after/ftplugin files. However, this causes the double-sourcing that you're seeing, which I suppose hasn't been reported before because ftplugin is chronically underused by most people.

As to fixing this: packadd hypothetically doesn't source ftplugin files, though I'll need to check that the docs match actual behavior here. That means that packer still needs to source a plugin's ftplugin files if they exist.

The solution here is probably to stop blindly triggering FileType and instead, at compile time, check if a plugin has anything in ftplugin, etc. and manually source these paths. The downside to that approach is that if a plugin adds ftplugin files in an update between compiles, they won't be found, causing misbehavior and pointing to yet another reason that I really need to find time to refactor packer to auto-recompile.