feat: add NullLsToggle command#1188
Conversation
lua/null-ls/init.lua
Outdated
| nargs = 1, | ||
| complete = function() | ||
| local list = {} | ||
| for _, source in ipairs(M.get_sources()) do |
There was a problem hiding this comment.
Should this complete with sources active for the current filetype?
There was a problem hiding this comment.
I made it for all registered sources for the current filetype, since it needs to re-enable them again.
| complete = function() | ||
| local list = {} | ||
| for _, source in ipairs(M.get_sources()) do | ||
| list[#list + 1] = source.name |
There was a problem hiding this comment.
I think there's a possibility of duplicate sources here, e.g. if the user has eslint registered for both formatting and diagnostics.
There was a problem hiding this comment.
this will depend on how source.toggle() should behave, and since it's only eslint, I think it's probably fine to leave it like this?
otherwise, we'd need something similar to how LspStop handles nargs
There was a problem hiding this comment.
If we just pass eslint to source.toggle() it'll toggle all sources matching the given query, which in this case means it'll toggle all sources named eslint. I think this would be the least surprising behavior for users, and there's at least a few other sources that share names, so I think we should deduplicate them.
lua/null-ls/info.lua
Outdated
| "Inactive source(s)", | ||
| } | ||
|
|
||
| for _, source in ipairs(sources.get(ft)) do |
There was a problem hiding this comment.
I haven't taken a close look, but is this logic okay? When I have multiple active sources and toggle them, I only see the first source under Inactive source(s).
There was a problem hiding this comment.
@jose-elias-alvarez, sorry for the delay! turned out to be two separate issues.. 😄
It should be fixed now!
jose-elias-alvarez
left a comment
jose-elias-alvarez
left a comment
There was a problem hiding this comment.
Sorry for the review back-and-forth on this! I found one bug but other than that I think this looks good.
| sources_info = create_active_sources_info(filetype) | ||
| logger_info = create_logging_info() | ||
| end | ||
| inactive_sources_info = create_inactive_sources_info(filetype) |
There was a problem hiding this comment.
I think both of these should also be under the is_attached condition, right? At the moment the create_logging_info() call is causing an error for me when the client isn't attached.
2 participants
add
NullLsTogglecommand with completion support