Improve: preserve begin-end keywords in if-then-else by Julow · Pull Request #978 · ocaml-ppx/ocamlformat

Julow · 2019-08-20T15:33:46Z

src/Params.ml

gpetiot · 2019-08-22T16:02:45Z

Regressions with test_branch.sh:

for if-then-else=k-r (and both values of exp-grouping):

diff --git a/infer/src/clang/Capture.ml b/infer/src/clang/Capture.ml
index 15f0c1bf3..4b184e114 100644
--- a/infer/src/clang/Capture.ml
+++ b/infer/src/clang/Capture.ml
@@ -190,6 +190,7 @@ let cc1_capture clang_cmd =
     L.(debug Capture Quiet) "@\n Skip the analysis of source file %s@\n@\n" source_path ;
     (* We still need to run clang, but we don't have to attach the plugin. *)
     run_clang clang_cmd Utils.consume_in
+  
   ) else if
       Config.skip_analysis_in_path_skips_compilation && CLocation.is_file_blacklisted source_path
     then (
diff --git a/compiler/lib/base64.ml b/compiler/lib/base64.ml
index 7d4999ed1..345b335dc 100644
--- a/compiler/lib/base64.ml
+++ b/compiler/lib/base64.ml
@@ -184,8 +184,7 @@ let decode_sub ?(pad = true) {dmap; _} ?(off = 0) ?len input =
     let n' = n // 4 * 3 in
     let res = Bytes.create n' in
     let get_uint8_or_padding =
-      if pad then (
-        fun t i ->
+      if pad then (fun t i ->
       if i >= len then raise Out_of_bounds;
       get_uint8 t (off + i)
       ) else

with if-then-else=compact (and both values of exp-grouping):

diff --git a/lambda/matching.ml b/lambda/matching.ml
index a9c4929fa..afba50ba8 100644
--- a/lambda/matching.ml
+++ b/lambda/matching.ml
@@ -1210,7 +1210,7 @@ and split_no_or cls args def k =
           assert (rev_no = []);
           let yes = List.rev rev_yes in
           insert_split group_discr yes (cl :: rem) def k
-          )
+        )
         else collect group_discr rev_yes (cl :: rev_no) rem
     | [] ->
         let yes = List.rev rev_yes and no = List.rev rev_no in
@@ -1388,7 +1388,7 @@ let split_and_precompile argo pm =
     Format.eprintf "** SPLIT **\n";
     pretty_pm pm;
     pretty_precompiled_res next nexts
-    );
+  );
   (next, nexts)
 
 (* General divide functions *)
@@ -2318,7 +2318,7 @@ let reintroduce_fail sw =
           if c > !max then (
             i_max := i;
             max := c
-            ))
+          ))
         t;
       if !max >= 3 then
         let default = !i_max in
@@ -2482,7 +2482,7 @@ let mk_failaction_pos partial seen ctx defs =
     Format.eprintf "**POS**\n";
     Default_environment.pp defs;
     ()
-    );
+  );
   let rec scan_def env to_test defs =
     match (to_test, Default_environment.pop defs) with
     | [], _
@@ -2518,9 +2518,9 @@ let mk_failaction_pos partial seen ctx defs =
     if dbg then (
       eprintf "POSITIVE JUMPS [%i]:\n" (List.length fail_pats);
       Jumps.eprintf jmps
-      );
+    );
     (None, fail, jmps)
-    )
+  )
   else (
     (* Too many non-matched constructors -> reduced information *)
     if dbg then eprintf "POS->NEG!!!\n%!";
@@ -2532,7 +2532,7 @@ let mk_failaction_pos partial seen ctx defs =
         | Some lam -> string_of_lam lam
         );
     (fail, [], jumps)
-    )
+  )

it looks better but shouldn't the closing parens be at the end of the previous line when if-then-else=compact?

Other combinations of exp-grouping and if-then-else are fine.

Julow · 2019-08-26T16:44:15Z

Regressions fixed.

gpetiot · 2019-08-27T12:27:08Z

Regressions fixed.

They are fixed for if-then-else=k-r but I still get them for if-then-else=compact.

Julow · 2019-08-27T13:33:04Z

Looks like this behavior is intended and this PR fixes it. I added a test for it: test/passing/ite-compact_closing.ml.ref

gpetiot

Looks good to me now.

CHANGES: #### Changes + Set "conventional" as the default profile (ocaml-ppx/ocamlformat#1060) (Guillaume Petiot) This new profile is made to better match the most used style and is encouraged. To continue using the previous default, use `profile = ocamlformat` in your `.ocamlformat`. + CLI: Allow both values of boolean options (ocaml-ppx/ocamlformat#1062) (Jules Aguillon) Now, both `--opt` and --no-opt` are available on the CLI for any boolean option "opt". Previously, only one of them were available depending on the default value. + Auto mode for `break-string-literals` (ocaml-ppx/ocamlformat#1057) (Guillaume Petiot) `wrap`, `newlines` and `newlines-and-wrap` values of `break-string-literals` are removed. `auto` replaces them, it is equivalent to `newlines-and-wrap`. + Dock collection brackets (ocaml-ppx/ocamlformat#1014) (Guillaume Petiot) `after-and-docked` value of `break-separators` is removed and is replaced by a new `dock-collection-brackets` option. + Preserve `begin` and `end` keywords in if-then-else (ocaml-ppx/ocamlformat#978) (Jules Aguillon) Previously, `begin`/`end` keywords around if-then-else branches were turned into parentheses. #### New features + Give a hint when warning 50 is raised (ocaml-ppx/ocamlformat#1111) (Guillaume Petiot) + Add a message when a config value is removed (ocaml-ppx/ocamlformat#1089) (Etienne Millon) Explain what replaces removed options and avoid printing a parsing error. + Implement `sequence-blank-line=preserve-one` for let bindings (ocaml-ppx/ocamlformat#1077) (Jules Aguillon) Preserve a blank line after `let .. in` when `sequence-blank-line` set to `preserve-one`. Previously, only blank lines after `;` could be preserved. + Parse toplevel directives (ocaml-ppx/ocamlformat#1020) (Jules Aguillon) Allow `#directives` in `.ml` files. Previously, files containing a directive needed to be parsed as "use file". The "use file" mode is removed and `--use-file` is now the same as `--impl`. + Don't require `--name`, require kind, forbid `--inplace`, allow `--check`, make `--enable-outside-detected-project` implicit when reading from stdin (ocaml-ppx/ocamlformat#1018) (Guillaume Petiot) + Parse code in docstrings (ocaml-ppx/ocamlformat#941) (Guillaume Petiot) Format OCaml code in cinaps-style comments `(*$ code *)` and code blocks in documentation comments `(** {[ code ]} *)`. + Parse documentation comments with Odoc (ocaml-ppx/ocamlformat#721) (Jules Aguillon) Formatting of documentation comments is more robust and support newer Odoc syntaxes. Internally, Odoc replaces Octavius as the documentation parser. #### Bug fixes + Fix unstabilizing comments on assignments (ocaml-ppx/ocamlformat#1093) (Guillaume Petiot) + Fix the default value documentation for `max-indent` (ocaml-ppx/ocamlformat#1105) (Guillaume Petiot) + Fix closing parenthesis exceeding the margin in function application (ocaml-ppx/ocamlformat#1098) (Jules Aguillon) + Missing break before attributes of `Pmty_with` (ocaml-ppx/ocamlformat#1103) (Josh Berdine) + Fix closing quote exceeding the margin (ocaml-ppx/ocamlformat#1096) (Jules Aguillon) + Fix break before the closing bracket of collections (exceeding the margin) (ocaml-ppx/ocamlformat#1073) (Guillaume Petiot) + Fix precedence of Dot wrt Hash (ocaml-ppx/ocamlformat#1058) (Guillaume Petiot) + Fix break in variant type definition to not exceed the margin (ocaml-ppx/ocamlformat#1064) (Guillaume Petiot) + Fix newlines and indentation in toplevel extension points (ocaml-ppx/ocamlformat#1054) (Guillaume Petiot) + Fix placement of doc comments around extensions (ocaml-ppx/ocamlformat#1052) (Jules Aguillon) + Inline extensions that do not break (ocaml-ppx/ocamlformat#1050) (Guillaume Petiot) + Add missing cut before attributes in type declarations (ocaml-ppx/ocamlformat#1051) (Guillaume Petiot) + Fix alignment of cases (ocaml-ppx/ocamlformat#1046) (Guillaume Petiot) + Fix blank line after comments at the end of lists (ocaml-ppx/ocamlformat#1045) (Guillaume Petiot) + Fix indexing operators precedence (ocaml-ppx/ocamlformat#1039) (Jules Aguillon) + Fix dropped comment after infix op (ocaml-ppx/ocamlformat#1030) (Guillaume Petiot) + No newline if the input is empty (ocaml-ppx/ocamlformat#1031) (Guillaume Petiot) + Fix unstable comments around attributes (ocaml-ppx/ocamlformat#1029) (Guillaume Petiot) + Fix extra blank line in sequence (ocaml-ppx/ocamlformat#1021) (Jules Aguillon) + Check functor arguments when computing placement of doc comments (ocaml-ppx/ocamlformat#1013) (Jules Aguillon) + Fix indentation of labelled args (ocaml-ppx/ocamlformat#1006) (Guillaume Petiot) + Fix linebreak between or-cases with comments when `break-cases=all` (ocaml-ppx/ocamlformat#1002) (Guillaume Petiot) + Fix unstable unattached doc comment in records (ocaml-ppx/ocamlformat#998) (Jules Aguillon) + Fix string literal changed (ocaml-ppx/ocamlformat#995) (Jules Aguillon) + Fix type variable (ocaml-ppx/ocamlformat#996) (Jules Aguillon) + Fix crash on extension sequence (ocaml-ppx/ocamlformat#992) (Guillaume Petiot) + Fix position of expressions regarding of comments in infix-op expressions (ocaml-ppx/ocamlformat#986) (Guillaume Petiot) + Escape special characters in external declaration (ocaml-ppx/ocamlformat#988) (Jules Aguillon) + Fix parens around constrained expr with attrs (ocaml-ppx/ocamlformat#987) (Guillaume Petiot) + Fix the margin, and correctly breaks comments (ocaml-ppx/ocamlformat#957) (Guillaume Petiot) + Fix formatting of custom indexing operators (ocaml-ppx/ocamlformat#975) (Guillaume Petiot) + Fix position of comments of labelled arrow types (ocaml-ppx/ocamlformat#976) (Guillaume Petiot) + No box around inline odoc styles (ocaml-ppx/ocamlformat#971) (Guillaume Petiot) + Fix boxing of collection expressions/patterns (ocaml-ppx/ocamlformat#960) (Guillaume Petiot) + Fix crash on record expr with pack fields (ocaml-ppx/ocamlformat#963) (Jules Aguillon) + Fix letop in subexpr (ocaml-ppx/ocamlformat#956) (hhugo) #### Internal + Take file kind from --name when formatting stdin (ocaml-ppx/ocamlformat#1119) (Jules Aguillon) + Make Fmt.t abstract (ocaml-ppx/ocamlformat#1109) (Jules Aguillon) + Future-proof Fmt API in case Fmt.t goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Future-proof `Fmt` API in case `Fmt.t` goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Optional names for formatting boxes in debug output (ocaml-ppx/ocamlformat#1083) (Guillaume Petiot) + Check ocamlformat error codes in the testsuite (ocaml-ppx/ocamlformat#1084) (Etienne Millon) + Clean `Translation_unit` (ocaml-ppx/ocamlformat#1078) (Guillaume Petiot) + Use dune file generation in test/passing/dune (ocaml-ppx/ocamlformat#1082) (Etienne Millon) + CI: factorize tests and check reason build (ocaml-ppx/ocamlformat#1079) (Guillaume Petiot) + Use short form for action in src/dune (ocaml-ppx/ocamlformat#1076) (Etienne Millon) + Cleanup `sequence_blank_line` (ocaml-ppx/ocamlformat#1075) (Jules Aguillon) + CI: use a script travis-ci.sh to simplify .travis.yml (ocaml-ppx/ocamlformat#1063) (Guillaume Petiot) + Remove utility functions from `Fmt_ast` (ocaml-ppx/ocamlformat#1059) (Guillaume Petiot) + CI: use opam-2.0.5 in Travis (ocaml-ppx/ocamlformat#1044) (Anton Kochkov) + CI: check the build with OCaml 4.07.1 and 4.08.0 (ocaml-ppx/ocamlformat#1036) (Jules Aguillon) + Use the same sets of options for both branches by default in `test_branch.sh` (ocaml-ppx/ocamlformat#1033) (Guillaume Petiot) + Fix `test_branch.sh` and CI checking of CHANGES.md (ocaml-ppx/ocamlformat#1032, ocaml-ppx/ocamlformat#1034) (Jules Aguillon) + Fix flag of git-worktree in `test_branch.sh` and `bisect.sh` (ocaml-ppx/ocamlformat#1027) (Guillaume Petiot) + Remove the `bisect_ppx` dependency and clean the `Makefile` (ocaml-ppx/ocamlformat#1005) (Jules Aguillon) + Use a `CHANGES.md` log file again (ocaml-ppx/ocamlformat#1023) (Guillaume Petiot) + Support OCaml 4.09.0 (add the odoc.1.4.2 dependency) (ocaml-ppx/ocamlformat#1024) (Guillaume Petiot) + Update labels of issue templates (ocaml-ppx/ocamlformat#1017) (Guillaume Petiot) + Update labels in `CONTRIBUTING.md` (ocaml-ppx/ocamlformat#1007) (Guillaume Petiot) + Allow to ignore invalid options (ocaml-ppx/ocamlformat#984) (hhugo) The `--ignore-invalid-option` flag is added to ignore invalid options in `.ocamlformat` files. + Improve the documentation of `--doc-comments` (ocaml-ppx/ocamlformat#982) (Jules Aguillon) + Remove symbolic links and change naming convention of tests (ocaml-ppx/ocamlformat#980) (Guillaume Petiot) + Change the type of `fmt_code` (ocaml-ppx/ocamlformat#974) (Guillaume Petiot) + Simplify `Makefile` (ocaml-ppx/ocamlformat#973) (hhugo) + Dune should not be flagged as a build dep anymore (ocaml-ppx/ocamlformat#954) (Guillaume Petiot)

Julow requested a review from gpetiot August 20, 2019 15:33

facebook-github-bot added the CLA Signed label Aug 20, 2019

Julow commented Aug 20, 2019

View reviewed changes

src/Params.ml Outdated Show resolved Hide resolved

src/Params.ml Show resolved Hide resolved

gpetiot changed the title ~~Preserve begin-end in if-then-else~~ Improve: preserve begin-end keywords in if-then-else Aug 20, 2019

gpetiot added the Status/5-Awaiting-feedback label Aug 20, 2019

gpetiot reviewed Aug 21, 2019

View reviewed changes

src/Params.ml Outdated Show resolved Hide resolved

Julow requested a review from gpetiot August 22, 2019 13:54

gpetiot force-pushed the fix-exp-grouping-ite branch from 96bf7bb to fca5ffa Compare August 22, 2019 14:19

Julow force-pushed the fix-exp-grouping-ite branch from 7df7bcb to d754ed9 Compare August 26, 2019 16:39

Add tests

ea5e540

Julow force-pushed the fix-exp-grouping-ite branch from d754ed9 to 8e28223 Compare August 26, 2019 16:42

Julow and others added 3 commits August 26, 2019 18:44

Preserve begin-end in if-then-else

8e28223

simplify

fb85052

make fmt

3a508a4

Julow force-pushed the fix-exp-grouping-ite branch from 0ad043f to e0db841 Compare August 27, 2019 13:32

Add tests for if-then-else=compact

e0db841

gpetiot approved these changes Aug 28, 2019

View reviewed changes

Julow merged commit cd5f7bf into ocaml-ppx:master Aug 28, 2019

Julow mentioned this pull request Nov 4, 2019

[new release] ocamlformat (0.12) ocaml/opam-repository#15208

Merged

bogdan2412 pushed a commit to bogdan2412/ocamlformat that referenced this pull request Mar 28, 2020

Improve: preserve begin-end keywords in if-then-else (ocaml-ppx#978)

f3982cf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve: preserve begin-end keywords in if-then-else#978

Improve: preserve begin-end keywords in if-then-else#978
Julow merged 5 commits intoocaml-ppx:masterfrom
Julow:fix-exp-grouping-ite

Julow commented Aug 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gpetiot commented Aug 22, 2019

Uh oh!

Julow commented Aug 26, 2019

Uh oh!

gpetiot commented Aug 27, 2019

Uh oh!

Julow commented Aug 27, 2019

Uh oh!

gpetiot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Julow commented Aug 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gpetiot commented Aug 22, 2019

Uh oh!

Julow commented Aug 26, 2019

Uh oh!

gpetiot commented Aug 27, 2019

Uh oh!

Julow commented Aug 27, 2019

Uh oh!

gpetiot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants