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

enable_outside_detected_project=true for rpc #2487

Closed
wants to merge 1 commit into from

Conversation

kohlivarun5
Copy link

The rpc api doesn't allow for enabling ocamlformat outside detected project.
In situations where we want the editor (ocaml-lsp) to respect the global config (in ~/.config/.ocamlformat), it is useful to enable formatting without having a .ocamlformat in the project

This change enables it for rpc, with the assumption that if a client is triggering the rpc explicitly for formatting, they expect it to be picked up

Related to #2092

Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

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

The lsp doesn't know whether formatting should be enabled or not and this will format files that shouldn't be in projects that do not use ocamlformat.

false is the right default for now as we decided not to implement #2092 yet (related #2439).
Instead, this should be an option that the lsp could be configured to pass or the lsp could handle specially projects that do not have a .ocamlformat.

@kohlivarun5
Copy link
Author

Instead, this should be an option that the lsp could be configured to pass or the lsp could handle specially projects that do not have a .ocamlformat.

Is it possible for the lsp to control this on the client side ?
My understanding of the code was that this option is not exposed and just hard-coded

@Julow
Copy link
Collaborator

Julow commented Jan 4, 2024

The lsp cannot pass this option at the moment and that's what we should fix.

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.

2 participants