Skip to content
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

feat: get_available_servers now accepts a filter #25

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

kylo252
Copy link
Contributor

@kylo252 kylo252 commented Aug 3, 2022

Description

Allow get_available_servers to accept a query filter

The available keys are

  • filetype (string): Only return servers with matching fileyupe

related: williamboman/mason.nvim#86

How has this been tested?

local all_servers = get_available_servers()
local js_servers = get_available_servers({ ft = "javascript" })

TODOs

  • this current returns the full list of servers if the filetype query fails (regardless if the filetype is actually valid from the start)
  • this should probably be used in parse_packages_from_heuristics
  • we could probably use an is_valid_server(), see the test-case in 358f627

@kylo252
Copy link
Contributor Author

kylo252 commented Aug 3, 2022

@williamboman, this one will be a bit more tricky, but I think it's quite useful, see williamboman/mason.nvim#86

P.s. I've spent way too long trying to get the annotations for this in a reasonable state, but I'm not sure how you'd want it to be, so feel free to add any suggestions.

@williamboman
Copy link
Owner

P.s. I've spent way too long trying to get the annotations for this in a reasonable state, but I'm not sure how you'd want it to be, so feel free to add any suggestions.

They look good to me! I changed the signature to allow both a string as well as a list of strings, felt more natural to me to allow both variants.

this should probably be used in parse_packages_from_heuristics

Good catch! Should be easy to incorporate, will look into it

Copy link
Owner

@williamboman williamboman left a comment

Choose a reason for hiding this comment

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

LGTM! Let's look into the TODOs in later PRs. I'll wait with merge until I have your blessing @kylo252, not sure if I messed something up with my changes

@williamboman williamboman merged commit e48a41e into williamboman:main Aug 5, 2022
vantaboard pushed a commit to vantaboard/mason-lspconfig.nvim that referenced this pull request Jan 10, 2024
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.

None yet

3 participants