Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

feat(cspell): Option to run cspell diagnostics with a specific cspell config #1329

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

juanpprieto
Copy link

@juanpprieto juanpprieto commented Jan 6, 2023

Context

Using cspell code_action's find_json function to reference a common cspell.json config across different projects works great. I can add 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 a cspell.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:

  • a find_json option to the diagnostics config (same as code_action)
  • a -c flag to the cspell command so that diagnostics are generated using the correct config
-- Use a common cspell.json across all projects
...
code_action.cspell.with({
  config = {
    find_json = function()
      -- add new words always to ~/cspell.json
      return vim.fn.findfile("cspell.json", vim.fn.environ().HOME .. ";")
    end,
  },
}),
diagnostics.cspell.with({
  config = {
    -- new
    find_json = function()
      -- create diagnostics using the config found at ~/cspell.json
      return vim.fn.findfile("cspell.json", vim.fn.environ().HOME .. ";")
    end,
  },
}),

@juanpprieto juanpprieto changed the title Option to run cspell diagnostics from a specific cspell config WIP: Option to run cspell diagnostics from a specific cspell config Jan 6, 2023
@juanpprieto juanpprieto changed the title WIP: Option to run cspell diagnostics from a specific cspell config feat(cspell): Option to run cspell diagnostics from a specific cspell config Jan 6, 2023
@@ -13,27 +13,6 @@ local cspell_diagnostics = function(bufnr, lnum, cursor_col)
return diagnostics
end

local CSPELL_CONFIG_FILES = {
Copy link
Author

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",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = {
Copy link
Author

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?

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.

@juanpprieto juanpprieto changed the title feat(cspell): Option to run cspell diagnostics from a specific cspell config feat(cspell): Option to run cspell diagnostics with a specific cspell config Jan 6, 2023
Copy link
Owner

@jose-elias-alvarez jose-elias-alvarez left a 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",

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 = {

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.

@jose-elias-alvarez
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants