Skip to content

Feat: Diagnostics table preprocess option.#21

Closed
coreyb-git wants to merge 4 commits intoravibrock:mainfrom
coreyb-git:feat_preprocess
Closed

Feat: Diagnostics table preprocess option.#21
coreyb-git wants to merge 4 commits intoravibrock:mainfrom
coreyb-git:feat_preprocess

Conversation

@coreyb-git
Copy link
Copy Markdown
Contributor

Added opts.func_preprocess(table) for users who want to dig through the SpellWarn diagnostics and change anything before they are sent to vim.diagnostic.set.

Note: This PR also includes the buftype PR code.

end

-- Pre-process, if a function is set in opts to do anything.
diags = opts.func_preprocess(diags)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, would check length or nil before calling following.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we also pass the bufnr to it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I figured if they change it from the default and it breaks then that's on them, and it should break so that they get hit with the errors.

Would you silently let if fail if it isn't assigned to a function? Or do some other one-time alert? I'm not sure the best way to go about this other than just let it fall on its face.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is not just about avoiding accidental errors; in fact, users may want to filter out all diagnostics entirely.

I'm not sure if vim.diagnostic.set can handle nil or {} correctly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we also pass the bufnr to it?

Probably a good idea.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is not just about avoiding accidental errors; in fact, users may want to filter out all diagnostics entirely.

I'm not sure if vim.diagnostic.set can handle nil or {} correctly.

I'm at the tail-end of a long day/night... let me know a bit more specifically what you mean, if possible?

Copy link
Copy Markdown

@chaneyzorn chaneyzorn Mar 10, 2026

Choose a reason for hiding this comment

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

let me know a bit more specifically what you mean, if possible?

My consideration is to include return nil or zero length (empty list) as part of the contractual agreement for this function interface, to express the intent of "users wanting to silence all diagnostics".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in the other PR I've altered it so that nil is converted to an empty table, where an empty table clears the diagnostics.

Comment on lines 19 to 23
local ft = vim.fn.getbufvar(bufnr, "&filetype")
if opts.ft_config[ft] == false or (opts.ft_config[ft] == nil and opts.ft_default == false) then
vim.diagnostic.reset(namespace, bufnr)
return
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

migrate to can_update?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'll just close both PR's and start a new one from my main branch that has the updates from both... this was fixed in the other PR, but is left-behind here by mistake.

-- Pre-process, if a function is set in opts to do anything.
diags = opts.func_preprocess(diags)

-- TODO: Add suffix diagnostics with type of spelling error the way that LSP diagnostics do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I just see this here, if suffix present, I won't need different prefix. thx.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure I follow... that's your own code, or from some older PR :P

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just realized you aren't ravi... my bad. Regardless, that TODO is not a part of anything I've done. You'll need to ask ravibrock about that.

Copy link
Copy Markdown

@chaneyzorn chaneyzorn Mar 10, 2026

Choose a reason for hiding this comment

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

haha, Just realized you are not ravibrock too.
Yes, I requested a feat issue about that.

Have a good rest. I would help reviewing some code in PR #22.

@coreyb-git coreyb-git marked this pull request as draft March 10, 2026 03:01
@coreyb-git coreyb-git closed this Mar 10, 2026
@coreyb-git
Copy link
Copy Markdown
Contributor Author

Replaced by PR #22

@coreyb-git coreyb-git deleted the feat_preprocess branch March 10, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants