ayamir / nvimdots

A well configured and structured Neovim.
BSD 3-Clause "New" or "Revised" License
2.91k stars 458 forks source link

fix: don't add the disabled plugins to plugins list. #1079

Closed ayamir closed 10 months ago

ayamir commented 10 months ago

1077

@Jint-lzxy @CharlesChiuGit @YuCao16

YuCao16 commented 10 months ago

1077 @Jint-lzxy @CharlesChiuGit @YuCao16

It works, thanks.

By the way, I noticed that all the user plugins' config are stored in user/configs/*.lua, but since I got too many additional plugins, the user/configs/ directory got messy, can we introduce the structures just as the /modules/configs/, for example user/configs/ui/*.lua?

YuCao16 commented 10 months ago

@ayamir @CharlesChiuGit I found a problem, this method seems not work for dependencies plugin, for example nvim-treesitter/nvim-treesitter, I cannot disable the dependencies such as andymass/vim-matchup.

Let's say I don't use andymass/vim-matchup, and I need another dependencies anuvyklack/pretty-fold.nvim. So in user/plugins/editor, I add following code (not sure this is the correct way?):

editor["nvim-treesitter/nvim-treesitter"] = {
    lazy = true,
    build = function()
        if #vim.api.nvim_list_uis() ~= 0 then
            vim.api.nvim_command([[TSUpdate]])
        end
    end,
    event = "BufReadPre",
    config = require("editor.treesitter"),
    dependencies = {
                {" anuvyklack/pretty-fold.nvim" },
        { "andymass/vim-matchup" },
        { "mfussenegger/nvim-treehopper" },
        { "nvim-treesitter/nvim-treesitter-textobjects" },
        {
            "abecodes/tabout.nvim",
            config = require("editor.tabout"),
        },
        {
            "windwp/nvim-ts-autotag",
            config = require("editor.autotag"),
        },
        {
            "NvChad/nvim-colorizer.lua",
            config = require("editor.colorizer"),
        },
        {
            "hiphish/rainbow-delimiters.nvim",
            config = require("editor.rainbow_delims"),
        },
        {
            "nvim-treesitter/nvim-treesitter-context",
            config = require("editor.ts-context"),
        },
        {
            "JoosepAlviste/nvim-ts-context-commentstring",
            config = require("editor.ts-context-commentstring"),
        },
    },
}

and set disabled_plugins = {"andymass/vim-matchup"}, but andymass/vim-matchup still enabled as current solution don't modify the dependencies.

ayamir commented 10 months ago

@YuCao16 It will remove plugins in dependenices now.

ayamir commented 10 months ago

By the way, I noticed that all the user plugins' config are stored in user/configs/.lua, but since I got too many additional plugins, the user/configs/ directory got messy, can we introduce the structures just as the /modules/configs/, for example user/configs/ui/.lua?

It can be done in another PR.

Jint-lzxy commented 10 months ago

By the way, I noticed that all the user plugins' config are stored in user/configs/*.lua, but since I got too many additional plugins, the user/configs/ directory got messy, can we introduce the structures just as the /modules/configs/, for example user/configs/ui/*.lua?

It can be done in another PR.

I think we already support this now:

editor["m4xshen/autoclose.nvim"] = {
    lazy = true,
    event = "InsertEnter",
    config = require("configs.editor.autoclose"), -- will require user/configs/editor/autoclose.lua
}

Let's say I don't use andymass/vim-matchup, and I need another dependencies anuvyklack/pretty-fold.nvim. So in user/plugins/editor, I add following code (not sure this is the correct way?):

Because lazy.nvim will also (internally) merge the plugin spec it manages, you only need to write:

editor["nvim-treesitter/nvim-treesitter"] = {
    dependencies = {
                { "anuvyklack/pretty-fold.nvim" },
        { "andymass/vim-matchup", enabled = false },
    },
}
ayamir commented 10 months ago

Hmm I'm a bit opposed to this PR b/c it not only adds a layer of complexity, but also addresses only one potential "edge use case". This is why folke said in this issue that he won't support including two plugins with the same (plugin) name but different repo names in the spec at the same time. Instead, he suggested that ppl should use the dev (or url) option, e.g.:

completion["nvimdev/lspsaga.nvim"] = {
  url = "https://example.com/path/to/ur/lspsaga.nvim.git",
  -- other configs
}

But seems it requires users to change repo's config. And I think we should keep our API for users works correctly for all of use cases.

Jint-lzxy commented 10 months ago

Hmm I'm a bit opposed to this PR b/c it not only adds a layer of complexity, but also addresses only one potential "edge use case". This is why folke said in this issue that he won't support including two plugins with the same (plugin) name but different repo names in the spec at the same time. Instead, he suggested that ppl should use the dev (or url) option, e.g.:

completion["nvimdev/lspsaga.nvim"] = {
    url = "https://example.com/path/to/ur/lspsaga.nvim.git",
    -- other configs
}

But seems it requires users to change repo's config. And I think we should keep our API for users works correctly for all of use cases.

I just checked the implementation of lazy.nvim, and it appears that when several plugins with the same name don't include conflicting URLs, lazy will use the source declared by the last appended specification (i.e., the spec provided by the user). The following minimal example demonstrates this:

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
    vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/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)

-- install plugins
local plugins = {
    "folke/tokyonight.nvim",
    -- add any other pugins here
    { "nvimdev/lspsaga.nvim" },
    { "ayamir/lspsaga.nvim", opts = {} },
}
require("lazy").setup(plugins, {
    root = root .. "/plugins",
})

vim.opt.termguicolors = true
vim.cmd([[colorscheme tokyonight]])

Save it as init.lua, and running it with nvim -u init.lua yields the following (possible) result:

  ● lspsaga.nvim 12.42ms ▷ start
      dir    /private/var/folders/jy/91xg_8j9169dydct2k4r7yw80000gn/T/tmp.chC1vsz66n/.repro/plugins/lspsaga.nvim
      url    https://github.com/ayamir/lspsaga.nvim
                                ^^^^^^
      branch main
      commit d3dfaea
      readme README.md
      help   |lspsaga.nvim.txt|

So IMO our loader has indeed done what it was supposed to do. The only thing missing here is we need to document this particular use case in the wiki - if users want to use their own fork(s) to overwrite the default one(s), they don’t need to include the base plugin(s) in disabled_plugins.

ayamir commented 10 months ago

OK, IMO this pr and #1077 can be closed now. @YuCao16