Fix regression in detecting project's root#1180
Conversation
|
Hi Julow! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
The value of --name was used as the starting point when searching for .ocamlformat files.
This behavior has been removed in #1127
Independent of other considerations like editors, this itself seems like a bad idea. What is the motivation for this change?
|
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
This wasn't intentional. I just found out about this behavior today. |
I don't think so, they should either use project detection from ocamlformat or from dune build rules. Otherwise this means that it'll have to be implemented in editors, which doesn't sound great? |
I agree. Editors shouldn't do that, they might not even know the root of the project. |
gpetiot
left a comment
There was a problem hiding this comment.
Looks good to me, thanks!
|
The tests look good to me, if you managed to reproduce the bug in emacs before the PR and it is fixed with this PR we can merge. |
46f4d35 to
eddbb4c
Compare
|
I think no changelog entry is needed as this fix a regression that hasn't been released. The fix works for me but I don't have a complete emacs setup. |
Fix #1139
The value of
--namewas used as the starting point when searching for.ocamlformatfiles.This behavior has been removed in #1127 but this breaks editor integration.
The emacs plugin calls ocamlformat like that:
ocamlformat --name src/my_file.ml /tmp/some_tmp_dir/some_tmp_file.ml.This PR restore this behavior and add a test. Editors should probably use the
--rootoption but it seems broken, any idea ?