feat/fix/break: rewrite of packer.nvim

[lewis6991]

Complete rewrite of packer with the focus of:

• an improved architecture
• strongly typed data structures
• module dependency graph with no cycles
• (greatly) reduced feature set for easier maintenance

User facing Changes

• requires Neovim 0.8
• spec.config now supports upvalues
• combined spec.cond and spec.disable into spec.enable
• renamed spec.setup to spec.config_pre
• removed:
  • use (*1). Plugin spec must now be given as a single table.
  • all compilation and PackerCompile
  • spec.module (*1)
  • luarocks
  • spec.as (*1)
  • spec.bufread
  • spec.rtp
  • spec.installer
  • spec.updater
  • spec.after
  • spec.fn
  • profiling (*2)
  • API
  • snapthots (to be replaced with lockfiles)
• config is greatly reduced

Internal Changes

• Ported to Teal
• Completely re-architected
• New async module (developed from gitsigns.nvim)
• removed result.lua and greatly simplified how job results are handled.
• all tests are broken (*2)

Future Changes

• Make all plugins opt by default and packer.startup call packadd for each plugin. This will make packer behave more similar to vim-plug
• Provide an interface similar to mason.nvim
• Bring up the testsuite

*1: Subject to being re-added *2: Plan to fix/add

[EdenEast]

Is this the planned rewrite of packer? Does this mean that I should port the lockfile PR #1010 to this?. Would this also be a good opportunity to remove snapshots as this is already a simplification of packer and its features?

[lewis6991]

Let's fence the lockfile stuff for now since the goal atm is to get a robust core. However this would be a good chance to remove snapshots if lockfiles are to replace them (which I think they should).

[bangedorrunt]

@lewis6991 may I know the reason why you remove module and module_pattern keys for lazyloading plugin?

[lewis6991]

The same reason as all the other removed features; to reduce the feature set to a core set that have the most benefit.

[bangedorrunt]

The same reason as all the other removed features; to reduce the feature set to a core set that have the most benefit.

Correct me if I'm wrong but module is essential feature, I couldn't find an alternative. I thought requires could be enough but not, if a plugin is lazyloaded, requires doesn't load it when needed.

[EdenEast]

This PR's state should be moved to draft while there still looks like there is development on it. Would help to clarify the current state of the PR.

[lewis6991]

The branch it is being merged into main will also be very alpha for a while. This will be merged when @wbthomason has time to review it. When it is merged, similar dev commits will continue to be made on the main branch.

[EdenEast]

Ah ok thanks for clarifying.

[lewis6991]

I ignored all the Lua files because I assume they are all generated from Teal now. If I'm wrong about this, please let me know and I'll go back over them.

Yes, Lua files are all generated and even marked so in .gitattributes.

Also, since we can't use selene anymore - is there an equivalent for Teal for non-type issues (things like style, performance, etc.)?

Unfortunately not. There is very little tooling for Teal. This is why I don't use it for every plugin, but I think the trade-offs for this project significantly favour using it. In Gitsigns I haven't found lack of style checking to be a problem, however, I'm the main contributor to that project.

On a similar note, I've recently been putting a bit of work into https://github.com/teal-language/teal-language-server so that should soon become more useful to us. A basic goto-definition was recently added, and I've got a change in flight that improves it.

I've left comments, but mostly nothing that I think blocks merging (some typo fixes, some places where I think we shouldn't hardcode the set of spec keys supported, etc.). Please take a look when you get a chance, and, if you agree, then I'm OK with merging.

Replied to all comments, I'll fix up what I can, then merge. Going forward, we should probably make a project board on github to track development.

After the merge, I'll convert my config to use the subset of features this branch supports and start dogfooding for a while. I can also contribute implementations of the features I requested be re-implemented already (if we agree that they belong - thinking specifically of setup under a new name, some manner of sequencing, use, profiling, and maaaaybe as/module/extensible handlers), and we can discuss if there's anything else that needs to be done (for the code - obviously, docs will need to be updated, I'll have to make a migration guide, etc.)

Awesome. As you've seen, the change overall is still quite rough in places. I've not tested this at all from a clean setup, so there will probably be a few hiccups to begin with.

I think I'm going to bring up the testsuite fairly soon. If you don't mind, I would like to replicate what I've done on Gitsigns and Impatient and re-use the framework Neovim core uses, and not use plenary.

Thank you again!

You're welcome!

[lewis6991]

I've actioned all the comments. I'll wait for any responses, otherwise we can merge soon.

[wbthomason]

Yes, Lua files are all generated and even marked so in .gitattributes.

So they are! Sorry, wrote that part of my review before looking at .gitattributes.

Unfortunately not. There is very little tooling for Teal. This is why I don't use it for every plugin, but I think the trade-offs for this project significantly favour using it.

Unfortunate indeed - ideally, I would like some way of imposing format/style standards. But I think you're right about the benefits of typing - if only for the Plugin type, even.

Going forward, we should probably make a project board on github to track development.

https://github.com/users/wbthomason/projects/1/views/1 (@lewis6991 and I have write access; everyone has read access)

I think I'm going to bring up the testsuite fairly soon. If you don't mind, I would like to replicate what I've done on Gitsigns and Impatient and re-use the framework Neovim core uses, and not use plenary.

Yes, I think moving away from plenary will be an improvement.

Looking through the responses now.

[wbthomason]

Ah, I also noticed that you removed util.float. I'm fine not having that be in util, but that was added because of a surprising (to me) and strong amount of user request. I think it's sufficiently commonly desired by users that it's worth keeping around somewhere - maybe in a contrib/extras module that does not get the same support as the "core", but includes useful extensions?

[delphinus]

I want to re-add setup absolutely for old Vim plugins. For example, dhruvasagar/vim-table-mode needs its configs to be done before loading the plugin itself.

require("packer").startup {
  {
    "dhruvasagar/vim-table-mode",
    config = function()
      vim.g.table_mode_corner = "|" --> it's too late!
    end,
  },
}

So without setup, we should write configs before packer.startup()

vim.g.table_mode_corner = "|"
-- and other settings……

require("packer").startup {
  "dhruvasagar/vim-table-mode",
}

The config and the entry are separated and it is too difficult to maintenance.

With the current packer, we can write as below.

require("packer").startup(function(use)
  use {
    "dhruvasagar/vim-table-mode",
    setup = function()
      vim.g.table_mode_corner = "|"
    end,
  }
end)

Can you to support this with PR?

[lewis6991]

Ah, I also noticed that you removed util.float. I'm fine not having that be in util, but that was added because of a surprising (to me) and strong amount of user request. I think it's sufficiently commonly desired by users that it's worth keeping around somewhere - maybe in a contrib/extras module that does not get the same support as the "core", but includes useful extensions?

How would you feel about make floating window the default? I'm very much a fan of the interface provided by https://github.com/williamboman/mason.nvim

[lewis6991]

@delphinus I've added spec.config_pre which is what spec.setup used to be.

[bangedorrunt]

@lewis6991 please consider readd spec.module. Using module, lazyloaded dependency could only be called when needed. I think it's essential feature. Thank you.

[lewis6991]

@lewis6991 please consider readd spec.module. Using module, lazyloaded dependency could only be called when needed. I think it's essential feature. Thank you.

With respect, whether you think it's essential isn't a good enough reason to justify its inclusion.

Like @delphinus did, please put forward a valid use case and explain the real benefits it provides.

[delphinus]

@lewis6991 I also use module for many purposes.

use {
  "mfussenegger/nvim-treehopper",
  module = { "tsht" },
  setup = function()
    vim.keymap.set("o", [['t]], [[:<C-u>lua require'tsht'.nodes()<CR>]], { remap = true })
  end,
}

This makes 't to load nvim-treehopper lazily because it uses require'tsht' in its mapping. Yes, we can do the same thing without module, but the code is hard to understand.

use {
  "mfussenegger/nvim-treehopper",
  key = { { "o", "'t" } },
  config = function()
    local tsht = require "tsht"
    -- We should write mappings again.
    vim.keymap.set("o", [['t]], function()
      tsht.nodes()
    end)
  end,
}

For another example, module makes telescope's config easy.

use {
  "nvim-telescope/telescope.nvim",
  module = { "telescope" },
  setup = function()
    local function builtin(name)
      return function(opt)
        return function()
          return require("telescope.builtin")[name](opt or {})
        end
      end
    end

    vim.keymap.set("n", "<Leader>f:", builtin "command_history" {})
    vim.keymap.set("n", "<Leader>fG", builtin "grep_string" {})
    vim.keymap.set("n", "<Leader>fH", builtin "help_tags" { lang = "en" })
    vim.keymap.set("n", "<Leader>fh", builtin "help_tags" {})
    vim.keymap.set("n", "<Leader>fm", builtin "man_pages" { sections = { "ALL" } })
  end,
}

By module, we can add many flexible mappings for many plugins.

[lewis6991]

use {
 "mfussenegger/nvim-treehopper",
 module = { "tsht" },
 setup = function()
   vim.keymap.set("o", [['t]], [[:<C-u>lua require'tsht'.nodes()<CR>]], { remap = true })
 end,
}

This should just be:

use {
  "mfussenegger/nvim-treehopper", config = function()
    vim.keymap.set("o", [['t]], function()
      require'nvim-treehopper'.nodes()
    end)
  end,
}

Lazy loading gets you very little here (15 LOC) and not worth the extra complexity to packer.

use {
 "nvim-telescope/telescope.nvim",
 module = { "telescope" },
 setup = function()
   local function builtin(name)
     return function(opt)
       return function()
         return require("telescope.builtin")[name](opt or {})
       end
     end
   end

   vim.keymap.set("n", "<Leader>f:", builtin "command_history" {})
   vim.keymap.set("n", "<Leader>fG", builtin "grep_string" {})
   vim.keymap.set("n", "<Leader>fH", builtin "help_tags" { lang = "en" })
   vim.keymap.set("n", "<Leader>fh", builtin "help_tags" {})
   vim.keymap.set("n", "<Leader>fm", builtin "man_pages" { sections = { "ALL" } })
 end,
}

Same applies here. There's no need to lazy load telescope. It's only lazy lazy-loading 142 LOC

You are much better off simply using https://github.com/lewis6991/impatient.nvim which I see is not in your config, so I'm not sure why you are so concerned with this kind of lazy-loading.

[wbthomason]

How would you feel about make floating window the default? I'm very much a fan of the interface provided by https://github.com/williamboman/mason.nvim

Works for me. I don't foresee much (if any) complaint about that.

[delphinus]

@lewis6991

Lazy loading gets you very little here (15 LOC) and not worth the extra complexity to packer.

This is an example for use cases, so it is not the problem that even the code is short as 15 lines or 142 lines. When the plugin I want to load lazily has 1000+ lines in the plugins's init.lua, do you think it is worth to re-add module into packer?

You are much better off simply using https://github.com/lewis6991/impatient.nvim which I see is not in your config, so I'm not sure why you are so concerned with this kind of lazy-loading.

You are right that impatient.nvim can reduce such delay. But I have satisfied with the current packer with no impatient.nvim. So it does not become the solution for this.

---

One more example for module to be re-added.

I want to load rcarriga/nvim-notify lazily until it calls vim.notify.

use {
  "rcarriga/nvim-notify",
  module = { "notify" },
  setup = function()
    vim.notify = function(...)
      local args = { ... }
      vim.notify = require "notify"
      vim.notify(unpack(args))
    end,
  end,
  config = function()
    require("notify").setup {
      -- some settings
    }
  end,
}

Yes, without module, we can write to do the same thing.

use {
  "rcarriga/nvim-notify",
  opt = true,
  setup = function()
    vim.notify = function(...)
      local args = { ... }
      vim.cmd.packadd "nvim-notify"
      local notify = require "notify"
      notify.setup {
        -- some settings
      }
      vim.notify = notify
      vim.notify(unpack(args))
    end,
  end,
}

But this code is truly ideal method to achieve it? Configs for nvim-notify is included in setup. And we must call packadd itself. I think the code with module is more readable and more maintenancable, isn't it?

[max397574]

you could just use vim.cmd.PackerLoad instead of packadd I think this makes it as readable as the other code

his is an example for use cases, so it is not the problem that even the code is short as 15 lines or 142 lines. When the plugin I want to load lazily has 1000+ lines in the plugins's init.lua, do you think it is worth to re-add module into packer?

in addition to that many people call the setup function inside the config so if a plugin isn't lazyloaded much more than just things inside /plugin is loaded e.g. commands are created. That often loads lots of code (e.g. utils or config)

[lewis6991]

I want to load lazily has 1000+ lines in the plugins's init.lua, do you think it is worth to re-add module into packer?

You are right that impatient.nvim can reduce such delay. But I have satisfied with the current packer with no impatient.nvim. So it does not become the solution for this.

These statements contradict each other.

Impatient gives you speed for every single Lua module for free! Even for very large Lua files. No need to architect any framework for lazy loading.

If you're satisfied with the current packer, then I don't understand your stake in this discussion. Just continue to use the current packer. The master branch isn't introducing any breaking changes.

I want to load rcarriga/nvim-notify lazily until it calls vim.notify.

This is the worst possible example! Lazy loading via module literally gets you NOTHING!

Just do this.

use {
  "rcarriga/nvim-notify",
  config = function()
    vim.notify = function(...)
      local notify = require "nvim-notify"
      notify.setup {
        -- some settings
      }
      vim.notify = notify
      vim.notify(...)
    end,
  end,
}

And in fact nvim-notify should probably do this anyway, so you would probably benefit the project by raising a PR that does this.

[delphinus]

@lewis6991

Sorry, you're right for nvim-notify's example. And I understood almost your intention for this changes. I had a fear of merging this breaking changes soon, but @wbthomason also says he takes more careful consideration before merging all changes into the master. That's good and I'm keeping to watch from now on.

BTW, thank you for great work for refactoring packer. 👍🏼

[yioneko]

Just do this.

use {
  "rcarriga/nvim-notify",
  config = function()
    vim.notify = function(...)
      local notify = require "nvim-notify"
      notify.setup {
        -- some settings
      }
      vim.notify = notify
      vim.notify(...)
    end,
  end,
}

This exmaple doesn't actually lazy load nvim-notify because files in plugin/ will be sourced automatically. If one file in init.vim make a call like require('nvim-notify'), this example just makes no sense.

Also the setup should be a one-shot call and side effects free to work in this example. Unfortunately, that's not true for most of the plugins now. Otherwise, I have to do something like this:

[gegoune]

Out of interest. How long does it take to load and set up nvim-notify with impatient installed?

[lewis6991]

This exmaple doesn't actually lazy load nvim-notify because files in plugin/ will be sourced automatically.

nvim-notify doesn't not have a plugin/dir.

If one file in init.vim make a call like require('nvim-notify'), this example just makes no sense.

Emm yes, that is always true, lazy-loading or not. Your comment makes no sense.

Also the setup should be a one-shot call and side effects free to work in this example. Unfortunately, that's not true for most of the plugins now. Otherwise, I have to do something like this:

It is one-shot. Read it again. Setup will only ever be called once.

This is way too much verbose and personally I cannot bear this simply because of the lack of support of module.

Good thing you don't need to do that as you can use my suggestion which is simpler.

[yioneko]

nvim-notify doesn't not have a plugin/dir.

I certainly know that but nvim-notify is just an example... There are plenty of plugins having it.

Emm yes, that is always true, lazy-loading or not. Your comment makes no sense.

I cannot understand what you mean. Please give more clarifications for it?

It is one-shot. Read it again. Setup will only ever be called once.

vim.notify is not a one-shot function. What I said is every time I call vim.notify, notify.setup {} will be triggered either.

@wbthomason Thanks I get it.

[WilliamHsieh]

vim.notify is not a one-shot function. What I said is every time I call vim.notify, notify.setup {} will be triggered either.

After the initial call to vim.notify, the entry for vim.notify is replaced with require("nvim-notify") in this line.

vim.notify = notify

So the setup function will only be called once.

[lewis6991]

I certainly know that but nvim-notify is just an example... There are plenty of plugins having it.

That's what I've been asking for. So far people have only provided bad examples.

I cannot understand what you mean. Please give more clarifications for it?

If you run requires in init.vim it will load a plugin. Lazy-loading doesn't help with that. Maybe I didn't understand your initial comment but it doesn't make sense to me.

[yioneko]

That's what I've been asking for. So far people have only provided bad examples.

If what needed is the "real" example that must require module, I'd agree there is none because user can always work around it by tricks like metatable. The only missing part is plugin/ will be auto sourced if is not marked as opt.

If you run requires in init.vim it will load a plugin. Lazy-loading doesn't help with that.

I think module could help in case like require in init.vim with the least efforts at user side.

BTW, I still think the example for nvim-notify is not a valid solution, because one could cache the initial notify function like this:

local notify = vim.notify
notify(...) -- this will setup nvim-notify
notify(...) -- this also calls setup because the function is cached

I know there are other methods to deal with it, but module is still the approach with the least mental overhead and best readability. I agree it is not necessary here, but I still expect a more ergonomic way for lazy loading.

[williamboman]

edit: spec.enable to the rescue! I think the general point regarding ergonomics and future extensibility of a single table still stands though.

use (*1). Plugin spec must now be given as a single table.

How would you optionally use certain plugins based on some arbitrary condition? With the "old" API this was pretty simple through normal control structures, for example:

if vim.fn.has "unix" == 1 then
    use { "unix-plugin1", "unix-plugin2", "unix-plugin3" }
end

I feel like the ergonomics to achieve the same through a single table becomes significantly worse?

Should the setup() function support falsey values in the table, people would end up with something like this, no?

require("packer").setup {
    vim.fn.has "unix" == 1 and "unix-plugin1",
    vim.fn.has "unix" == 1 and "unix-plugin2",
    vim.fn.has "unix" == 1 and "unix-plugin3",
}

[max397574]

I think it should still be possible to make multiple calls to the function (as it is now with use)

[clason]

Use if foo then packer.add({...}) end; see the open PR.