-
Notifications
You must be signed in to change notification settings - Fork 5
Feat: Diagnostics table preprocess option. #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
68b7630
a9d1866
30a826a
73d4012
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,26 +50,41 @@ function M.update_diagnostics(opts, bufnr) | |
| end | ||
| end | ||
| end | ||
|
|
||
| -- 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. haha, Just realized you are not ravibrock too. Have a good rest. I would help reviewing some code in PR #22. |
||
| vim.diagnostic.set(namespace, bufnr, diags, opts.diagnostic_opts) | ||
| end | ||
|
|
||
| local function can_update(opts, bufnr) | ||
| local winid = vim.api.nvim_get_current_win() | ||
| if winid then | ||
| if not vim.wo[winid].spell then | ||
| vim.diagnostic.reset(namespace, bufnr) -- ensure old are cleared if spell is toggled to off. | ||
| return | ||
| end | ||
| end | ||
|
|
||
| local buftype = vim.api.nvim_get_option_value("buftype", { buf = bufnr }) | ||
| if opts.bt_config[buftype] then | ||
| return opts.bt_config[buftype] | ||
| end | ||
|
|
||
| return opts.bt_default | ||
| end | ||
|
|
||
| function M.setup(opts) | ||
| function M.enable() | ||
| vim.api.nvim_create_augroup("Spellwarn", {}) | ||
| vim.api.nvim_create_autocmd(opts.event, { | ||
| group = "Spellwarn", | ||
| callback = function() | ||
| local winid = vim.api.nvim_get_current_win() | ||
| local bufnr = vim.fn.bufnr("%") | ||
| if winid then | ||
| if not vim.wo[winid].spell then | ||
| vim.diagnostic.reset(namespace, bufnr) -- ensure old are cleared if spell is toggled to off. | ||
| return | ||
| end | ||
| if can_update(opts, bufnr) then | ||
| M.update_diagnostics(opts, bufnr) | ||
| end | ||
|
|
||
| M.update_diagnostics(opts, bufnr) | ||
| end, | ||
| desc = "Update Spellwarn diagnostics", | ||
| }) | ||
|
|
||
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.
LGTM, would check length or nil before calling following.
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.
Should we also pass the bufnr to it?
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 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.
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.
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.
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.
Probably a good idea.
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 at the tail-end of a long day/night... let me know a bit more specifically what you mean, if possible?
Uh oh!
There was an error while loading. Please reload this page.
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.
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".
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.
in the other PR I've altered it so that nil is converted to an empty table, where an empty table clears the diagnostics.