use correct -short-paths config for merlin#1579
use correct -short-paths config for merlin#1579SkySkimmer wants to merge 1 commit intoocaml:masterfrom
Conversation
|
Not sure how adding tests works here but a pair of tests (with / without short paths) should probably be added. |
voodoos
left a comment
voodoos
left a comment
There was a problem hiding this comment.
Thanks ! It's not unreasonable, and would make ocaml-lsp's default behavior the same as merlin's.
I re-read Merlin's configuration processing, and I think I have a way to disable short-paths without modifying the default behavior here:
(env
(_
(flags ((:standard \ -short-paths) -real-paths))))
Could you try it ? Would you be happy with such a solution ? I am a bit hesitant to change the default behavior here, but we do need it to be configurable.
|
I've never heard of -real-paths, are you sure it's real? Indeed the problem as I understand it is that there's no opposite flag to -short-paths, so there's no way to explicitly get the default ocaml behaviour and we have to just not pass -short-paths. BTW it's the default behaviour of ocaml, not just merlin. |
|
|
voodoos
left a comment
voodoos
left a comment
There was a problem hiding this comment.
Let's move back to the same defaults as the rest of the stack.
@SkySkimmer can you rebase and add a changelog entry ?
Fix ocaml#1395 The default needs to be `real_paths = true` (from `Mconfig.initial`), because `get_external_config` (called in `config`) will modify the config from the ocaml flags which will not contain a negated `-short-paths` when short paths are not wanted.
|
It is done |
2 participants
Fix #1395
The default needs to be
real_paths = true(fromMconfig.initial), becauseget_external_config(called inconfig) will modify the config from the ocaml flags which will not contain a negated-short-pathswhen short paths are not wanted.Should backport cleanly to 414-LTS (or rather I tested on 414-LTS then moved the commit on top of master ;))