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

Bug: .ocamlformat-ignore glob patterns do not work on Windows #1773

Closed
nojb opened this issue Aug 23, 2021 · 5 comments · Fixed by #2206
Closed

Bug: .ocamlformat-ignore glob patterns do not work on Windows #1773

nojb opened this issue Aug 23, 2021 · 5 comments · Fixed by #2206

Comments

@nojb
Copy link
Contributor

nojb commented Aug 23, 2021

$ ls -a
.  ..  .ocamlformat  .ocamlformat-ignore  a.ml
$ cat .ocamlformat
$ cat .ocamlformat-ignore
*.ml
$ cat a.ml
let _ =
1
$ ocamlformat a.ml
let _ = 1 # BUG: a.ml should be ignored!

Note that this is only about glob patterns, literal filenames work fine (they were fixed in #1752).

@nojb
Copy link
Contributor Author

nojb commented Aug 23, 2021

Root cause seems to stem from ocaml-re, see ocaml/ocaml-re#197

@gpetiot
Copy link
Collaborator

gpetiot commented Sep 2, 2021

It looks like your fix is included in ocaml-re 1.10.0 https://github.com/ocaml/ocaml-re/releases/tag/1.10.0 if you confirm it's fixed we can add the re >= 1.10.0 dependency to ocamlformat

@nojb
Copy link
Contributor Author

nojb commented Sep 2, 2021

It looks like your fix is included in ocaml-re 1.10.0 https://github.com/ocaml/ocaml-re/releases/tag/1.10.0 if you confirm it's fixed we can add the re >= 1.10.0 dependency to ocamlformat

Thanks for the ping. Actually the fix included in 1.10.0 was only partial. A fuller fix was merged after that one in ocaml/ocaml-re#198 and will be included in the next release 1.10.1 (and will require a tiny patch to ocamlformat). I will open a PR here once 1.10.1 is out with the modification. Thanks!

cc @rgrinberg

@gpetiot
Copy link
Collaborator

gpetiot commented Aug 1, 2022

ocamlformat now requires ocaml-re.1.10.3 (#2133), are there still some Windows-specific things to fix or can we close this issue?

@nojb
Copy link
Contributor Author

nojb commented Aug 1, 2022

ocamlformat now requires ocaml-re.1.10.3 (#2133), are there still some Windows-specific things to fix or can we close this issue?

Hi. Sorry, this had dropped out of my radar. Yes, ocamlformat needed a tiny patch if I remember correctly. I will try to revisit this soon and submit a PR to ocamlformat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants