Saghen / blink.cmp

Performant, batteries-included completion plugin for Neovim
MIT License
1.49k stars 83 forks source link

Suggestion to simplify `source.provider` config #144

Closed chrisgrieser closed 1 month ago

chrisgrieser commented 1 month ago

The problem

As some issues (e.g., #132) or discussions on reddit show, the current way to configure sources appears to be confusing to some users. I think the problem derives from the fact that sources.providers is a list and not a dictionary.

Using lists of non-primitives is somewhat counterintuitive for many people, since the way vim.tbl_deep_extend() works, you need to copypaste the whole list if you want to adjust one property in one of the list elements. This leads to confusions, as can be seen in #132. Using lists of non-primitives is a pretty rare occurrence in nvim plugins to begin with, probably due to this problem.

Other than being less intuitive, the need to specify the full list unnecessarily creates the need to copy a lot of default configs, even if you intend to only change one detail. Furthermore, exposing "blink.cmp.sources.lsp" to the user feels unnecessary to me, just referring to a source via its name "LSP" should be enough?

Suggested change

I suggest using a dict to configure the individual sources, something like this:

sources = {
    enabled = { "LSP", "Snippets", "Path", "Buffer" },
    config = {
        LSP = {
            keyword_length = 0,
        },
        Snippets = {
            score_offset = -3,
            opts = {}
        },
        Path = {
            score_offset = 3,
            opts = {},
        },
        Buffer = {
            fallback_for = { "LSP" },
        }
    }
},

Comparison of the current config and the suggested change

If a user just wants to specify the path to their snippet folder, but not change anything else regarding blink.cmp's default config, their config currently will have to look like this:

sources = {
    providers = {
        { "blink.cmp.sources.lsp", name = "LSP" },
        { "blink.cmp.sources.path", name = "Path", score_offset = 3 },
        {
            "blink.cmp.sources.snippets",
            name = "Snippets",
            score_offset = -3,
            opts = {
                search_paths = { "/my/snippets" },
            },
        },
        { "blink.cmp.sources.buffer", name = "Buffer", fallback_for = { "LSP" } },
    }
},

With the suggested change, they only need to do this:

sources = {
    config = {
        Snippets = {
            opts = {
                search_paths = { "/my/snippets" },
            },
        },
    },
},
Saghen commented 1 month ago

Yeah, totally agree with you, I've got a few commits locally implementing this:

sources = {
  --- @type string[] | fun(ctx: blink.cmp.Context): string[]
  completion = { 'lsp', 'path', 'buffer', 'snippets' } 
  cmdline = { 'commands', 'path' }
  search = { 'buffer' },

  providers = {
    lsp = {
      --- @type string
      name = 'LSP',
      --- @type string
      module = 'blink.cmp.sources.lsp',

      --- @type fun(ctx: blink.cmp.Context)
      enabled = true,
      --- @type table
      opts = {}

      --- @type fun(ctx: blink.cmp.Context, items: blink.cmp.Item[]): blink.cmp.Item[]
      transform_items = nil,
      --- @type fun(ctx: blink.cmp.Context, items: blink.cmp.Item[]): boolean
      should_show_items = nil,
      --- @type number | fun(ctx: blink.cmp.Context, items: blink.cmp.Item[]): boolean
      max_items = nil,

      --- @type number
      min_keyword_length = nil,
      --- @type string[] | fun(ctx: blink.cmp.Context, enabled_sources: string[]): string[]
      fallback_for = {},
      --- @type number | fun(ctx: blink.cmp.Context, enabled_sources: string[]): number
      score_offset = 0,

      -- override any builtin function of the source
      override = { ... },
    },
    path = { ... },
    buffer = { ... },
    snippets = { ... }
  },
}

Not settled on this though so feedback is welcome

lopi-py commented 1 month ago

just giving some ideas

sources_list = {
  'lsp',
  'path',
  'snippets',
  'buffer',
},
sources = {
  snippets = {
    opts = {
      search_paths = { '/my/snippets' },
    },
  },
},

sources_list may be split into

-- is "auto" in "autocompletion" a bit too much?
sources_completion = { "lsp", "path", "snippet", "buffer" },
sources_cmdline = {
  [":"] = { "cmdline" },
  ["/"] = { "buffer" },
  ["?"] = { "buffer" }, 
},
sources_by_ft = {
  gitcommit = { "git" },
}

blink will search for sources in blink.cmp.sources.[source_name], so each source must have to define their own name (imo this feels more right), e.g.

local source = {}
source.name = "LSP"

function source.new()
  return setmetatable({}, { __index = source })
end