-
Notifications
You must be signed in to change notification settings - Fork 788
feat(cspell): Option to run cspell diagnostics with a specific cspell config #1329
base: main
Are you sure you want to change the base?
feat(cspell): Option to run cspell diagnostics with a specific cspell config #1329
Conversation
@@ -13,27 +13,6 @@ local cspell_diagnostics = function(bufnr, lnum, cursor_col) | |||
return diagnostics | |||
end | |||
|
|||
local CSPELL_CONFIG_FILES = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to reuse these between diagnostics and code_actions so I moved them to a shared cspell helper
local cspell_args = { | ||
"lint", | ||
"-c", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See -c
flag here
https://cspell.org/docs/getting-started/#options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, cspell
can run without a config file. What will happen when this flag is used but the config file doesn't exist?
@@ -0,0 +1,25 @@ | |||
local CSPELL_CONFIG_FILES = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like it might be the wrong place for this logic as it's not a global null-ls helper. I just couldn't find a more suitable folder. Maybe adding a common
or shared
folder within builtins/ would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree that if we're going to go with this approach we should put this elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this approach - I agree that there's an inconsistency between the diagnostics source and the code action source, but I think this is a lot of complexity to add for something that's already solved by extra_args
. There's also potential performance issues - running findfile()
each time the user requests code actions is okay, but running it on every change (which is what happens for diagnostics sources) could cause serious slowdown.
Since these sources are so closely linked, perhaps there's a better solution where one source can derive config values from the other. This might just be a case where we can add a config recipe to the wiki, though, since I think there's already solutions that use existing null-ls functionality.
local cspell_args = { | ||
"lint", | ||
"-c", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, cspell
can run without a config file. What will happen when this flag is used but the config file doesn't exist?
@@ -0,0 +1,25 @@ | |||
local CSPELL_CONFIG_FILES = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree that if we're going to go with this approach we should put this elsewhere.
FYI: I wonder if the approach used in #1339 could also help solve some of the inconsistencies between the code action source and the diagnostics source. |
Context
Using cspell code_action's
find_json
function to reference a commoncspell.json
config across different projects works great. I canadd
new words to the dict without any problem. However...The problem
The cspell
diagnostic
command is executed without a-c
(config) flag which makes it default to looking for acspell.json
file at the cwd.This means that diagnostics do not pickup changes (new words added) to the
cspell.json
file we wrote to via the code action.Solution
This PR adds:
find_json
option to the diagnostics config (same as code_action)-c
flag to thecspell
command so that diagnostics are generated using the correct config