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

feat: add NullLsToggle command #1188

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kylo252
Copy link
Contributor

@kylo252 kylo252 commented Oct 15, 2022

add NullLsToggle command with completion support

nargs = 1,
complete = function()
local list = {}
for _, source in ipairs(M.get_sources()) do

Choose a reason for hiding this comment

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

Should this complete with sources active for the current filetype?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

I think there's a possibility of duplicate sources here, e.g. if the user has eslint registered for both formatting and diagnostics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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/init.lua Outdated Show resolved Hide resolved
@kylo252 kylo252 marked this pull request as ready for review October 21, 2022 09:48
"Inactive source(s)",
}

for _, source in ipairs(sources.get(ft)) do

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jose-elias-alvarez, sorry for the delay! turned out to be two separate issues.. 😄

It should be fixed now!

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.

Sorry for the review back-and-forth on this! I found one bug but other than that I think this looks good.

end
inactive_sources_info = create_inactive_sources_info(filetype)

Choose a reason for hiding this comment

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

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.

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.

2 participants