cspell.nvim icon indicating copy to clipboard operation
cspell.nvim copied to clipboard

CSpell Multi Config Support

Open JateNensvold opened this issue 1 year ago • 1 comments
trafficstars

Hello! I recently started using this plugin within the last several months and came across a use case that is currently not supported by cspell.nvim. I have found that I often want to split words apart into global and local dictionaries so I can configure project repositories with individual cspell config files while also reducing repeated cspell configuration by using a config with words common across my development system.

Currently cspell.nvim does not support using multiple cspell.json files to enable configuration of dictionaries and custom words from multiple locations(ex. global config and local config) but this is a supported use case as shown in the cspell documentation

I have been dabbling around with adding support for this new feature and have a working feature branch that I submitted in https://github.com/davidmh/cspell.nvim/pull/62

It looks like this feature was discussed once before in https://github.com/davidmh/cspell.nvim/issues/18 and it would be great to hear any review/feedback on what would be required to merge this in, or any other thoughts on adding support for the usage of multiple configuration sources for cspell.nvim . Thanks!

JateNensvold avatar Jul 17 '24 03:07 JateNensvold

Thanks for the PR! I'll take a look later today.

davidmh avatar Jul 17 '24 16:07 davidmh

Is there a way to not have to use a project-local config file?

Currently, with this config:

local config = {
        cspell_config_dirs = {
            "~/.config/cspell",
        },
}

I get this merged config file:

{
  "flagWords": [],
  "import": [
    "/Users/jean/path/to/current/project/cspell.json",
    "/Users/jean/.config/cspell/cspell.json"
  ],
  "version": "0.2",
  "words": [],
  "language": "en"
}

Which then results in cspell erroring because my current project from which I'm running Neovim doesn't have a project-specific config file.

Here's the specific error I get:

[null-ls] failed to run generator: ...plugin-null-ls/lua/null-ls/helpers/generator_factory.lua:231: error in generator output: Configuration Configuration Loader Error: Failed to resolve configuration file: "/Users/jean/path/to/current/project/cspell.json" referenced from "/Users/jean/.cache/nvim/cspell.nvim/%Users%jean%path%to%current%project%cspell.json" ------------------------------------------- CSpell: Files checked: 0, Issues found: 0 in 0 files with 1 error.

edit I resolved this with echo "{}" > ~/cspell.json, but that feels like a workaround, as I have no need for that additional config file (which now also shows up in my code-action list).

JeanMertz avatar Aug 26 '24 10:08 JeanMertz

Hi @JeanMertz, thanks for the report, I'll look into it later this week.

davidmh avatar Aug 28 '24 17:08 davidmh

I won't have time to address this soon, I'm thinking about rolling back the feature to unblock existing users.

@JateNensvold what do you think?

davidmh avatar Aug 30 '24 21:08 davidmh

Alright, I've reverted for now, you should be able to use it as before, @JateNensvold I would love your input on this issue.

davidmh avatar Aug 30 '24 21:08 davidmh

I'm taking a look into it now, Ill let you know what I find.

JateNensvold avatar Aug 30 '24 21:08 JateNensvold

Sorry for the delay I wrote 90% of the following message 3 weeks ago, and then got pulled away to do some other things before I could finish.

I was able to reproduce the issue by opening up cspell.nvim in a project that does not have a local cspell.json file(or is missing the file returned by find_json).

There is a quick solution that I have tested that fixes this issue and unblocks diagnostics. The root cause is from the code for merging multiple cspell not checking if the config files exist before adding it into the merged config. So when a user has defined a list of global configs like the following

cspell_config_dirs = {
            "~/.config/cspell",
        },

The config merging code adds in CWD to keep backwards compatibility with cspell.nvim behavior prior to when multi config support was added.

With previous functionality when the file returned by

            local config_path = helpers.get_config_path(params)

did not exist it would get skipped while running diagnostics

            if config_path then
                cspell_args = vim.list_extend({ "-c", config_path }, cspell_args)
            end

As mentioned above the same functionality can be added for multi-config support by adding a filter to check for if the cspell config file exists before adding it to the merged config

    for _, cspell_config_path in pairs(cspell_config_mapping) do
        local path_exists = cspell_config_path ~= nil and cspell_config_path ~= "" and Path:new(cspell_config_path):exists()
        if path_exists then
            table.insert(cspell_config_paths, cspell_config_path)
        else
            local debug_message =
                M.format('Unable to find file at "${file_path}", skipping adding to merged cspell config.', { file_path = cspell_config_path })
            logger:debug(debug_message)
        end
    end

@JeanMertz If you also want to only get a single global config without CWD you could use find_json to return your main global config and leave cspell_config_dirs blank or use it for any additional global configs.

As for the other issue

as I have no need for that additional config file (which now also shows up in my code-action list

This is a bit trickier as it requires decoupling much of the previous logic in cspell.nvim that assumes the user always wants to use the config file at CWD(or returned by find_json).

I'm happy to take a stab at adding a feature flag(or another solution) over the next week(s) that allows CWD/find_json to be disable when opted into but would like to hear your thoughts @davidmh.

Ill publish my fix in a new PR to fix the behavior pior to when multi_config support was reverted, let me know if you if you have any concerns with the fix I outlined above or if you have thoughts on a better approach.

JateNensvold avatar Sep 18 '24 05:09 JateNensvold

Yeah, making the multi-config feature opt-in sounds like the ideal solution.

davidmh avatar Sep 19 '24 18:09 davidmh