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

fix: respect language settings of imported files

Open spacian opened this issue 10 months ago • 9 comments
trafficstars

cspell only respects the language of the main config, thus making it impossible to add new languages exclusively by importing them. This pull request appends all the languages found in imported files to the main config automatically.

Fixes #74

spacian avatar Jan 17 '25 13:01 spacian

One thing I noticed is that cspell normally allows trailing commas in json, this implementation does not. Maybe there is an easy way to fix that.

spacian avatar Jan 17 '25 13:01 spacian

Another thing I noticed is that now the config needs a way to be reset. I only deleted the files manually, but that does not sound like a great way to do things, as it needs to be done every time a language is added.

I think it would be good to expose a function that resets / deletes the internal cspell.json for the current working directory, but I am kind of new to this and unsure on how to do that. Then every user could invoke the call as they please. E.g. I would probably just clear the file on VimEnter or SessionEnter.

spacian avatar Jan 17 '25 17:01 spacian

I'm very thankful you're taking your time to look into this!

I see your point and I guess I'm mixing in a different issue... I think once the plugin is loaded, it just uses the first cspell file it found. However, I'm working a lot with sessions and when I switch my session, the plugin keeps using the old cspell.json. I feel like both of these things are connected and could be resolved simultaneously, but I could be wrong.

Just for my understanding, can params.cwd be different from vim.loop.cwd() or vim.fn.getcwd()? My naive mind thinks "the cwd is the cwd, no matter where I get it from."

An option to just force the rebuild every time (if that is what you mean) would probably be the easiest to implement. And at least on my machine, cspell takes like 1s to run anyway, another few milliseconds to rewrite a rather small file would hardly be noticeable.

Other than that I'll let my mind wander a little, maybe some other idea comes to mind. Have a nice weekend!

spacian avatar Jan 18 '25 07:01 spacian

I thought a little bit and also had to debug some stuff and I found out that the params.cwd never changes after it is set for the first time. This caused all the issues with the internal cspell.json not being updated on cwd changes, even when there was a new local cspell.json.

It also allowed me to introduce an option which forces a rewrite of the cspell.json when the global cwd changes. It is implemented by caching the last known working directory and forces a rewrite when the last know directory differs from the current working directory.

Last but not least I noticed occasionally that deep folder structures were generated for the internal cspell.json. I added another gsub to also replace \ with -. Now all the files are directly in the [cache_path]/cspell folder on windows with no further folders inside it, which looks correct to me, but maybe that was not your intention.

spacian avatar Jan 19 '25 08:01 spacian

Just for my understanding, can params.cwd be different from vim.loop.cwd() or vim.fn.getcwd()? My naive mind thinks "the cwd is the cwd, no matter where I get it from."

Not quite, params.cwd is set by the null-ls source, in case the directory containing the CSpell config file is different from Neovim's current working directory.

For example, working on a monorepo with the following structure:

.
├── cspell.json
├── service-a/
│   └── README.md
├── service-b/
│   └── README.md
└── service-c/
    └── README.md

You may want of focus on working on service-b, so that's where you open your Neovim instance, but you still want to use the CSpell config at the root of the repository.

For those cases, you can use the cwd property included in all null-ls sources. See: https://github.com/nvimtools/none-ls.nvim/blob/af318216b744f2fe2d5afa6febb01f455ae61d6c/doc/BUILTIN_CONFIG.md?plain=1#L293-L305

Using vim.loop.cwd or vim.fn.getcwd should be used as a fallback.

Using vim.fn.getcwd as the main cwd means that we would be ignoring that setting, and it also causes the code actions to break if I'm running in a directory different from the CSpell config file parent.

I can solve the issue by consuming cwd as:

local cwd = params.cwd or require('null-ls.utils').get_root()

I also noticed that changing the cwd keeps appending to the cached configuration, which ends up mixing configurations across projectds.

For example, working on the following projects:

.
├── project-a/
│   └── cspell.json
├── project-b/
│   └── cspell.json
└── project-c/
    └── cspell.json

Let's say I start by editing project-c, then I switch to work on project-a, by manually setting the cwd or using something like vim-rooter, now the code actions include options for both projects. And if I then open project-b, I'll get actions for all three projects.

That doesn't seem right.


Last but not least I noticed occasionally that deep folder structures were generated for the internal cspell.json. I added another gsub to also replace \ with -. Now all the files are directly in the [cache_path]/cspell folder on windows with no further folders inside it, which looks correct to me, but maybe that was not your intention.

I don't have a way to test against Windows, I would be comfortable with anything you decide on that, as long as it's covered by a test.

davidmh avatar Jan 19 '25 20:01 davidmh

Thank you for your in-depth feedback!

It looks like I have a lot to learn about the plugin still. I'll look into it more, especially what the caches are doing and how that will interact with resets.

I feel like the usage of the params.cwd currently contradicts my personal usage of sessions because I just open nvim anywhere and then open my session, which also sets the cwd. But then params.cwd is never changed after being set once, because it always finds itself in subsequent calls even when I open a different project (i.e. a different session) in the same nvim instance. I'll probably have to check for session switches to reset params.cwd. That might also be a good point to reset the caches, however that is done properly. I'll figure it out, hopefully.

Thank you again for your time and effort!

spacian avatar Jan 19 '25 21:01 spacian

My new approach is injecting reset_spell and cwd functions instead of exposing corresponding setters. This allows every user to be as flexible as they want with setting their cwd and resetting cspell. And while I was at it, I added the option also have default imports because I didn't want to always create a cspell.json for the German dictionary I'm using.

For example, I currently use this:

	local null_ls = require("null-ls")
	local cspell_last_session = ""
	null_ls.setup({
		fallback_severity = vim.diagnostic.severity.HINT,
		sources = {
			require("cspell").diagnostics.with({
				config = {
					cspell_import_files = function(_)
						return { vim.fn.expand("$APPDATA") .. "/npm/node_modules/@cspell/dict-de-de/cspell-ext.json" }
					end,
					cwd = function(_)
						return vim.fn.getcwd()
					end,
					reset_cspell = function(params)
						if params ~= nil and params.cwd ~= nil and vim.v.this_session ~= cspell_last_session then
							cspell_last_session = vim.v.this_session
							return true
						else
							return false
						end
					end,
				},
			}),
		},
	})

All the information in params is available because I pass params when calling cwd and reset_cspell. Additionally, I'm clearing the caches now on reset. Whether that solves the issues you described I still have to check.

I also just now figured out how to escape all the right characters on windows to get the tests to run at all. Now I have to debug my runtime errors and also figure out what to test and how.

spacian avatar Jan 25 '25 20:01 spacian

Unfortunately, I currently find very little time to work on this. I hope to in the future, but it's probably unrealistic at least in the next month or two. I'm not quite sure what to do with this merge request during that time. We could leave it as it is, I could open a new one when I think I made significant progress or convert it to a draft. Anything is fine by me, but I wanted to let you know.

spacian avatar Feb 07 '25 08:02 spacian

No worries, happy to review that new version whenever you're ready.

davidmh avatar Feb 07 '25 16:02 davidmh