Peter Szilagyi avatar Peter Szilagyi committed 3f1f3f3

Added ocp-indent tests for Tuareg indentation bugs.

Comments (0)

Files changed (21)

 ocaml/contrib/ocp-indent/config.status
 ocaml/contrib/ocp-indent/main.native
 ocaml/contrib/ocp-indent/src/globals.ml
+ocaml/contrib/ocp-indent/tests/js-*.1.ml
+ocaml/contrib/ocp-indent/tests/js-*.1.mlp
+ocaml/contrib/ocp-indent/tests/js-*.diff
 ocaml/omake/OMakefile
 ocp-indent/ocp.ml
 ocp-indent/tuareg.ml
Add a comment to this file

ocaml/contrib/ocp-indent/.git/index

Binary file modified.

ocaml/contrib/ocp-indent/.git/logs/HEAD

 0000000000000000000000000000000000000000 f4b6699209a061fa3044eb55caeed7c4fdc256cb Peter Szilagyi <pszilagyi@nyc-qws-147.delacy.com> 1350932476 -0400	clone: from https://github.com/OCamlPro/ocp-indent.git
+f4b6699209a061fa3044eb55caeed7c4fdc256cb 549407b47e945151fb89bf651516ac5af9c9b387 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352386363 -0500	commit: Added DIFFFLAGS so I can "make tests DIFFFLAGS=-u" to see context better.
+549407b47e945151fb89bf651516ac5af9c9b387 483da98555b2a9f93533204e23fdf7097093a989 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352387230 -0500	commit: sp.
+483da98555b2a9f93533204e23fdf7097093a989 2c4e1a24bcf1cf7a2b1f1ca8993ffbe3e5a8d121 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352387300 -0500	commit: Added tests based on Jane Street's internal Tuareg indentation bugs list.

ocaml/contrib/ocp-indent/.git/logs/refs/heads/master

 0000000000000000000000000000000000000000 f4b6699209a061fa3044eb55caeed7c4fdc256cb Peter Szilagyi <pszilagyi@nyc-qws-147.delacy.com> 1350932476 -0400	clone: from https://github.com/OCamlPro/ocp-indent.git
+f4b6699209a061fa3044eb55caeed7c4fdc256cb 549407b47e945151fb89bf651516ac5af9c9b387 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352386363 -0500	commit: Added DIFFFLAGS so I can "make tests DIFFFLAGS=-u" to see context better.
+549407b47e945151fb89bf651516ac5af9c9b387 483da98555b2a9f93533204e23fdf7097093a989 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352387230 -0500	commit: sp.
+483da98555b2a9f93533204e23fdf7097093a989 2c4e1a24bcf1cf7a2b1f1ca8993ffbe3e5a8d121 Peter Szilagyi <pszilagyi@tot-qws-017.delacy.com> 1352387300 -0500	commit: Added tests based on Jane Street's internal Tuareg indentation bugs list.

ocaml/contrib/ocp-indent/.git/refs/heads/master

-f4b6699209a061fa3044eb55caeed7c4fdc256cb
+2c4e1a24bcf1cf7a2b1f1ca8993ffbe3e5a8d121

ocaml/contrib/ocp-indent/tests/Makefile

 
 %.ml.indent: %.ml
 	@$(OCPINDENT) $*.ml > $*.1.ml
-	@(diff $*.ml $*.1.ml > $*.diff \
+	@(diff $(DIFFFLAGS) $*.ml $*.1.ml > $*.diff \
 	  && echo "OK     $*.ml" && rm $*.diff && rm $*.1.ml) \
 	  || echo  "ERROR  $*.ml"
 
 
 %.mlp.indent: %.mlp
 	@$(OCPINDENT) --lines 5-8 $*.mlp > $*.1.mlp
-	@(diff $*.mlp $*.1.mlp > $*.diff \
+	@(diff $(DIFFFLAGS) $*.mlp $*.1.mlp > $*.diff \
 	  && echo "OK     $*.mlp" && rm $*.diff && rm $*.1.mlp) \
 	  || echo  "ERROR  $*.mlp"
 
 clean:
-	rm -f *.diff *.1.ml *.2.ml *.1.mlp *.2.mlp
+	rm -f *.diff *.1.ml *.2.ml *.1.mlp *.2.mlp

ocaml/contrib/ocp-indent/tests/js-and.ml

+module M : S with type a = b
+              and type c = d
+;;

ocaml/contrib/ocp-indent/tests/js-andand.ml

+let all_equal =
+  a = b
+  && c = d
+  && e = f
+;;
+
+let _ =
+  x
+  && t.entity = entity
+  && t.clearing_firm = clearing_firm
+  && t.type_ = type_

ocaml/contrib/ocp-indent/tests/js-args.ml

+let () =
+  foo.bar <-
+    f x
+      y z
+
+let should_check_can_sell_and_marking regulatory_regime =
+  match z with
+  | `foo
+    -> some_function
+      argument
+
+let f = fun x -> g
+  x
+
+let z =
+  some_function
+    argument
+
+
+
+let () =
+  f a b ~c:c
+    d
+
+let () =
+  f a b ~c:1.
+    d
+
+let () =
+  My_module.f a b ~c:c
+    d
+
+let () =
+  My_module.f a b ~c:1.
+    d

ocaml/contrib/ocp-indent/tests/js-bind.ml

+let assigned_to u =
+  Deferred.List.filter (Request_util.requests ()) ~f:(fun request ->
+    if ...
+    then ...
+    else
+      status_request ~request () ~msg_client:no_msg >>= fun status ->
+      not (up_to_date_user status u))

ocaml/contrib/ocp-indent/tests/js-comment.ml

+(* ocp-indent is not going to be confused by comment-embedded tokens. *)
+
+
+
+type t = {
+  (* This is a comment *)
+  a: int;
+}
+
+type t = {
+  (* This is a comment : with a colon. *)
+  a: int;
+}
+
+type t = {
+  a: int;
+  (* with the :     second field *)
+  b: int;
+}
+
+type t = {
+  a: int;
+  b: int;
+  (* and : the third... *)
+  c: int;
+}
+
+
+
+(* colon in CR comment *)
+type cfg = {
+  foo : int;  (* CR mburns: float? *)
+  bar : string;
+}
+
+type t= {
+  foo : int;
+  bar : string; (* CR mburns: float? *)
+  baz : string;
+}
+
+type t= {
+  foo : int;
+  (* CR mburns: float? *)
+  bar : string;
+}

ocaml/contrib/ocp-indent/tests/js-fun.ml

+let _ =
+  [f (fun x ->
+     x);
+   f (fun x ->
+     x);
+   f (fun x ->
+     x);
+  ]
+;;
+
+let _ =
+  x
+  >>= fun x ->
+  (try x with _ -> ())
+  >>= fun x ->
+  try x with _ -> ()
+    >>= fun x ->
+    x
+;;

ocaml/contrib/ocp-indent/tests/js-functor.ml

+module M =
+  F (G)
+    (H) (* OK, line up the functor args *)
+
+module M =
+  F
+    (G)
+    (H)
+
+module M =
+  Functor (G)
+    (H)
+    (I)
+
+include F(struct
+  let blah ...
+end)
+  (G)

ocaml/contrib/ocp-indent/tests/js-let.ml

+let foo
+    some very long arguments that we break onto the next line
+  =
+  bar ();
+  baz

ocaml/contrib/ocp-indent/tests/js-map.ml

+let projection_files =
+  Deferred.List.map x ~f:(fun p ->
+    ...)
+  >>| String.split ~on:'\n'
+
+let projection_files = Deferred.List.map x ~f:(fun p ->
+  ...)
+  >>| String.split ~on:'\n'

ocaml/contrib/ocp-indent/tests/js-pattern.ml

+let f = function
+  | _ -> 0
+;;
+
+let f x = match x with
+  | _ -> 0
+;;
+
+let f =
+  function
+  | _ -> 0
+;;
+
+let f x =
+  match x with
+  | _ -> 0
+;;
+
+
+
+let check_price t = function
+    { Exec.
+      security_type = Some security_type;
+      symbol = Some full_symbol_hum;
+      currency = Some currency;
+      price = Some price;
+      trade_at_settlement = (None | Some false);
+      executing_exchange;
+      fill_id;
+    } -> ()
+
+let check_price t = function
+  | { Exec.
+      trade_at_settlement = (None | Some false);
+    } -> ()
+
+let check_price t = function
+  | simpler -> ()
+
+let check_price t = function
+    simpler -> ()
+
+let check_price t = function
+    simpler -> ()
+  | other -> ()

ocaml/contrib/ocp-indent/tests/js-pipebang.ml

+let f x =
+  x
+  >>| fun x ->
+  g x
+  >>| fun x ->
+  h x
+;;
+
+let f x =
+  x
+  |! fun x ->
+  g x
+  |! fun x ->
+  h x
+;;
+
+let f x =
+  x |! fun x ->
+  g x |! fun x ->
+  h x
+;;
+
+
+
+    (z (fun x -> x)
+     |! Validate.of_list)
+
+    (z x
+     |! Validate.of_list)

ocaml/contrib/ocp-indent/tests/js-poly.ml

+let handle_query qs ~msg_client:_ = try_with (fun () ->
+  if ... then
+    f >>| fun () ->
+    `Done ()
+  else
+    ...
+)
+;;
+
+if ... then ... else
+  assert_branch_has_node branch node >>| fun () ->
+  { t with node; floating; }
+;;

ocaml/contrib/ocp-indent/tests/js-record.ml

+let x =
+  { x with
+    foo = 3
+  ; bar = 5
+  }
+
+let x =
+  { (* blah blah blah *)
+    foo = 3
+  ; bar = 5
+  }
+;;

ocaml/contrib/ocp-indent/tests/js-sexp.ml

+let () =
+  f
+    x
+    <:sexp_of<int>>
+    y
+;;
+
+let z =
+  some_function
+    <:sexp_of<foo>>
+;;
+
+let z =
+  some_function
+    argument

tuareg-bugs/tuareg-indentation-bugs.ml

+(* These are exported, more or less, to ../ocaml/contrib/ocp-indent/tests/js-*.  Note that,
+   there, they are indented as preferred. *)
 
 module M : S with type a = b
              and type c = d  (* should be over one more space *)
     (G) (* Not great, probably should be indented more to the left *)
     (H)
 
+(* CR pszilagyi: To me, this looks fine as it is, and that's how I put it in
+   ../ocaml/contrib/ocp-indent/tests/js-functor.ml like this.  The rule seems fine as
+   "indent arguments by 2".  To illustrate, with a case where the functor name is
+   longer: *)
+module M =
+  Functor (G)
+    (H)
+    (I)
+
 include F(struct
   let blah ...
 end)
-  (G) (* What is G doing out here? *)
+    (G) (* What is G doing out here? *)
 
 (* ------------------------------------------------------------------------ *)
 
   ...)
                        >>| String.split ~on:'\n' (* Should be as above *)
 
+(* CR pszilagyi: I put this in ../ocaml/contrib/ocp-indent/tests/js-map.ml as suggested,
+   but here we come to a conflict between a couple of indentation principles.  I actually
+   think the immediately above should be implemented more like this: *)
+let projection_files = Deferred.List.map x ~f:(fun p ->
+  ...)		(* whole indentation scheme shifted to align *)
+                       >>| String.split ~on:'\n'
+(* The conflict is between the "indent only as far as necessary given nesting and
+   punctuation" and "align arguments to the right of the beginning of the expression".  In
+   this case, the first rule wants to make the "..." way over to the left, while the
+   second puts ">>|" way over to the left.  I think we favor the first rule, as suggested
+   before, so that's what I've put in ../ocaml/contrib/ocp-indent/tests/js-map.ml. *)
+
 (* -------------------------------------------------------------------------- *)
 
 module A = struct
 end
 (* Using '(modify-syntax-entry ?_ "_" js-tuareg-syntax-table)', the 'in' as a part of a
    longer identifier name messes up indentation. *)
+(* CR pszilagyi: This seems to have been fixed.  The syntax table seems to have been split
+   into jane-tuareg-indent-syntax-table and jane-tuareg-navigation-syntax-table and we
+   only mess with -indent-.  Anyway, this is not one that will confuse ocp-indent. *)
 module B = struct
   exception E_in
-           let () = ()
-             end
+  let () = ()				(* was aligned with "n E_in" *)
+end					(* was aligned with "E_in" *)
 
 (* -------------------------------------------------------------------------- *)
 
 
 let _ =
   [f (fun x ->
-    x);
+    x);					(* Should be 1 more right. *)
    f (fun x ->
      x); (* Why is this out space farther than two lines above? *)
    f (fun x ->
   (try x with _ -> ())
   >>= fun x ->
   try x with _ -> ()
-  >>= fun x ->
+  >>= fun x ->				(* Should be 2 more right. *)
     x (* Again, why is this farther out? *)
 ;;
 
   | `foo
     -> some_function
     argument                            (* should be indented 5 right *)
+(* CR pszilagyi: This is a bit in conflict with how we indent other function call
+   arguments in pattern-matches.  I'd claim it should be only 2 more right.  For example,
+   what would you say about this one: *)
+let f = fun x -> g
+  x
 (* so it would be more similar to this: *)
 let z =
   some_function
 
 (* -------------------------------------------------------------------------- *)
 
-The following code indents incorrectly:
+(* The following code indents incorrectly: *)
 
     (z (fun x -> x)
                  |! Validate.of_list)
 
-should be
+(* should be *)
 
     (z (fun x -> x)
      |! Validate.of_list)
 
-indeed, this does work correctly (if you drop the fun)
+(* indeed, this does work correctly (if you drop the fun) *)
 
     (z x
      |! Validate.of_list)
 
 (* -------------------------------------------------------------------------- *)
 
-Are you still collecting examples of non-optimal indentation in tuareg-mode? One that gets me from time to time is the following:
+(* Are you still collecting examples of non-optimal indentation in tuareg-mode? One that
+   gets me from time to time is the following: *)
 
 let foo
     some very long arguments that we break onto the next line
   bar ();
   baz
 
-The picture shows where I want the `=' to be. However tuareg currently moves it over to line up with the arguments, viz.:
+(* The picture shows where I want the `=' to be. However tuareg currently moves it over to
+   line up with the arguments, viz.: *)
 
 let foo
     some very long arguments that we break onto the next line
   bar ();
   baz
 
-Perhaps this is merely a personal preference, but that seems ugly to me.
+(* Perhaps this is merely a personal preference, but that seems ugly to me. *)
 
 (* -------------------------------------------------------------------------- *)
 
   executing_exchange;
   fill_id;
 }
+(* CR pszilagyi: This isn't complete, but, is the bug that the record (pattern) should be
+   indented 2 more right?  The principle isn't totally clear in my mind here; it's a
+   pattern, so it should be indented at the same level as a simpler pattern, so it should
+   actually be indented 4 more right (leaving room for the "|" that would be indented 2
+   right in a simpler pattern).  That's kind of a weird rule, though, when applied to
+   patterns without any "|", like this.
 
-  (* ---------------------------------------------------------- *)
+   Maybe we actually want the indentation to be different depending on the presence of the
+   "|"?  That might make it difficult for ocp-indent, since that will be
+   extra-grammatical.  I suppose ocp-indent already has that problem for the different
+   indentations of different binary operators (although maybe those are more bugs in
+   Tuareg than what we want).
+
+   Anyway, adding the result clause so it's syntactically correct: *)
+-> ()
+
+(* ---------------------------------------------------------- *)
 
 type t = {
   (* This is a comment *)
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.