{"action":"created","issue":{"url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/498","repository_url":"https://api.github.com/repos/ocaml-ppx/ocamlformat","labels_url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/498/labels{/name}","comments_url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/498/comments","events_url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/498/events","html_url":"https://github.com/ocaml-ppx/ocamlformat/pull/498","id":378802196,"node_id":"MDExOlB1bGxSZXF1ZXN0MjI5NDE5ODkz","number":498,"title":"Add a parens-tuple-patterns configuration option","user":{"login":"NathanReb","id":7419360,"node_id":"MDQ6VXNlcjc0MTkzNjA=","avatar_url":"https://avatars2.githubusercontent.com/u/7419360?v=4","gravatar_id":"","url":"https://api.github.com/users/NathanReb","html_url":"https://github.com/NathanReb","followers_url":"https://api.github.com/users/NathanReb/followers","following_url":"https://api.github.com/users/NathanReb/following{/other_user}","gists_url":"https://api.github.com/users/NathanReb/gists{/gist_id}","starred_url":"https://api.github.com/users/NathanReb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/NathanReb/subscriptions","organizations_url":"https://api.github.com/users/NathanReb/orgs","repos_url":"https://api.github.com/users/NathanReb/repos","events_url":"https://api.github.com/users/NathanReb/events{/privacy}","received_events_url":"https://api.github.com/users/NathanReb/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-08T16:05:20Z","updated_at":"2018-11-08T16:05:24Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/pulls/498","html_url":"https://github.com/ocaml-ppx/ocamlformat/pull/498","diff_url":"https://github.com/ocaml-ppx/ocamlformat/pull/498.diff","patch_url":"https://github.com/ocaml-ppx/ocamlformat/pull/498.patch"},"body":"Fixes #488\r\n\r\n## What's going on\r\n\r\nThe idea is to allow users to control how they want tuple patterns to be parenthesized.\r\n\r\nI added a configuration option called `parens-tuple-patterns` that can be `always` or `multi-line-only` just as it's expression counterpart and default to the latter for retro-compatibility reasons.\r\n\r\nI also updated the documentation for the `parens-tuple` option to clarify that it only applies to expressions as changing its name would break compatibility.\r\n\r\nThere's a new test case in `test/passing/parens_tuple_patterns.ml` to check both behaviour and the default one.\r\n\r\n## Tests results\r\n\r\nWhile `make test` passes, I had some issues with `tools/test_branch.sh`.\r\nRunning it with master gives the following output:\r\n```\r\n11% 81:641=8s code/infer/infer/src/checkers/NullabilitySuggest.ml                                                                                                                                                  ocamlformat.exe: Cannot process \"code/infer/infer/src/bufferoverrun/bufferOverrunModels.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n15% 109:613=5s code/infer/infer/src/checkers/Sanitizer.ml                                                                                                                                                          ocamlformat.exe: Cannot process \"code/infer/infer/src/checkers/loopInvariant.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n15% 110:612=5s code/infer/infer/src/checkers/SimpleChecker.ml                                                                                                                                                      ocamlformat.exe: Cannot process \"code/infer/infer/src/checkers/control.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n19% 141:581=4s code/infer/infer/src/backend/Tasks.ml                                                                                                                                                               ocamlformat.exe: Cannot process \"code/infer/infer/src/backend/OndemandCapture.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n19% 144:578=4s code/infer/infer/src/backend/Differential.ml                                                                                                                                                        ocamlformat.exe: Cannot process \"code/infer/infer/src/backend/reporting.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n30% 223:499=2s code/infer/infer/src/clang/cType_to_sil_type.ml                                                                                                                                                     ocamlformat.exe: Cannot process \"code/infer/infer/src/clang/ClangWrapper.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n39% 285:437=3s code/infer/infer/src/istd/Escape.ml                                                                                                                                                                 ocamlformat.exe: Cannot process \"code/infer/infer/src/istd/ARList.ml\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n56% 411:311=2s code/infer/infer/src/IR/Cfg.mli                                                                                                                                                                     ocamlformat.exe: Cannot process \"code/infer/infer/src/IR/ProcnameDispatcher.mli\".\r\n  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.\r\n  BUG: comments changed.\r\n100% 722:0=0s code/infer/sledge/src/ppx_trace/ppx_trace.mli                                                                                                                                                        \r\nmake[1]: *** [Makefile:121: test_inplace] Error 8\r\nmake[1]: Leaving directory '/home/nathan/code/ocamlformat/test'\r\nmake: *** [Makefile:43: test] Error 2\r\nmake: Leaving directory '/home/nathan/code/ocamlformat/test\r\n```\r\nWhich I assume is not the expected successful one. Running it with this branch gave the same results.\r\n\r\nNote that running it with the new option enabled by setting the `OCAMLFORMAT` en variable failed with a bunch of those:\r\n```\r\nFatal error: exception (\"User Error: malformed OCAMLFORMAT environment variable\"\r\n  (\"unknown option\" parens-tuple-patterns))\r\nRaised at file \"src/error.ml\" (inlined), line 9, characters 14-30\r\nCalled from file \"src/error.ml\", line 11, characters 19-40\r\nCalled from file \"src/Conf.ml\", line 1458, characters 8-462\r\nCalled from file \"src/Conf.ml\", line 1503, characters 4-85\r\nCalled from file \"src/Conf.ml\", line 1544, characters 56-74\r\nCalled from file \"src/list.ml\", line 322, characters 13-17\r\nCalled from file \"src/list.ml\" (inlined), line 350, characters 15-31\r\nCalled from file \"src/Conf.ml\", line 1543, characters 6-125\r\n```\r\nwhich is I guess expected since the master branch can't handle this option. Having an easy way to run those tests for new options (or documenting it if it exists) would help.\r\n\r\nDo you have any idea what's wrong here?\r\n\r\nI have to admit I had a bit of a hard time figuring out how to fix the tests, both from `make test` and `tools/test_branch.sh`. There's probably room for improvement in `CONTRIBUTING.md`, but I got them working in the end. I'm happy to help improving the documentation for contributors if you feel like that would be a worthy addition.\r\n\r\nAlso have you considered using `dune` and its `diff` action for the tests so that the diff would be printed for failing tests and `dune promote` would help contributors update the expected output? I feel like it would be a perfect match for `ocamlformat` tests and would definitely make it easier to work with but I might be missing some existing mechanisms.\r\n\r\nAnyway, I digress again, let me know what you think!"},"comment":{"url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/comments/437050869","html_url":"https://github.com/ocaml-ppx/ocamlformat/pull/498#issuecomment-437050869","issue_url":"https://api.github.com/repos/ocaml-ppx/ocamlformat/issues/498","id":437050869,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNzA1MDg2OQ==","user":{"login":"facebook-github-bot","id":6422482,"node_id":"MDQ6VXNlcjY0MjI0ODI=","avatar_url":"https://avatars3.githubusercontent.com/u/6422482?v=4","gravatar_id":"","url":"https://api.github.com/users/facebook-github-bot","html_url":"https://github.com/facebook-github-bot","followers_url":"https://api.github.com/users/facebook-github-bot/followers","following_url":"https://api.github.com/users/facebook-github-bot/following{/other_user}","gists_url":"https://api.github.com/users/facebook-github-bot/gists{/gist_id}","starred_url":"https://api.github.com/users/facebook-github-bot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/facebook-github-bot/subscriptions","organizations_url":"https://api.github.com/users/facebook-github-bot/orgs","repos_url":"https://api.github.com/users/facebook-github-bot/repos","events_url":"https://api.github.com/users/facebook-github-bot/events{/privacy}","received_events_url":"https://api.github.com/users/facebook-github-bot/received_events","type":"User","site_admin":false},"created_at":"2018-11-08T16:05:24Z","updated_at":"2018-11-08T16:05:24Z","author_association":"COLLABORATOR","body":"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 up 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 the corporate CLA signed.\n\nIf you have received this in error or have any questions, please contact us at [cla@fb.com](mailto:cla@fb.com?subject=CLA%20for%20ocaml-ppx%2Focamlformat%20%23498). Thanks!"}}
{"actor":{"display_login":"facebook-github-bot"}}