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

Support configurable completion candidates #315

Closed
majjoha opened this issue Jun 5, 2024 · 3 comments
Closed

Support configurable completion candidates #315

majjoha opened this issue Jun 5, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@majjoha
Copy link
Contributor

majjoha commented Jun 5, 2024

Firstly, thank you for an absolutely amazing piece of software!

In #233, @narutohahahaha asked if it would be possible to add support for allowing users to configure the number of completion candidates in the .marksman.toml configuration file. Is this something you'd consider?

If somebody wanted to try and implement it, where would they have to start? Would it be a matter of updating https://github.com/artempyanykh/marksman/blob/main/Marksman/Config.fs with an additional field for completion candidates, and then update this line to read the value from the configuration file (and if not set, default to 50) instead, or is there more to it than that?

@artempyanykh
Copy link
Owner

artempyanykh commented Jun 8, 2024

Hey @majjoha, thanks for the kind words!

Re: implementing the feature, I'd also add 'write a couple tests' 😉 but otherwise that'd be pretty much it.

@majjoha
Copy link
Contributor Author

majjoha commented Jun 12, 2024

Sounds good!

I've given it a go in #316, but as I have close to no prior experience with F# or LSPs, I could use a few pointers on how to finish the feature. Currently, the implementation seems to not read the candidates field in the configuration file correctly, as it still uses the default value which is now set to 50. Similarly, in the tests, Marksman.ConfigTests.testDefault and Marksman.ConfigTests.testParse_7 in ConfigTests.fs, the expected value is not matched, as the result is null for both tests.

Is there anything obvious that I am doing wrong? :-)

@artempyanykh
Copy link
Owner

Thanks for having a stab @majjoha ! I commented on the PR

@artempyanykh artempyanykh added the enhancement New feature or request label Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants