Commits

Peter Szilagyi  committed 11978dc

Moved most tuareg-indentation-bugs into contrib/ocp-indent/tests,
including full commentary, so we don't need to keep two copies.

  • Participants
  • Parent commits 33cc0e2

Comments (0)

Files changed (14)

 =======
 - Color "nonrec" as a keyword in OCaml.
 - Moved from /mnt/global/base/lib/elisp/{js,jane-test} to
-  /j/<office>/app/emacs/{prod,dev}/jane-elisp.
+  /j/office/app/emacs/{prod,dev}/jane-elisp.
 - Incorporated most of /mnt/global/base/lib/elisp into elisp/contrib.
 
 new stuff

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

Binary file modified.

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

+(* sehrlichman *)
 let all_equal =
   a = b
   && c = d
-  && e = f
+  && e = f (* this && should line up with previous one *)
 ;;
 
+(* ereisner: '=' seems to be relevant here *)
 let _ =
   x
   && t.entity = entity

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

     f x
       y z
 
+(* yminsky *)
 let should_check_can_sell_and_marking regulatory_regime =
   match z with
   | `foo
     -> some_function
       argument
-
+(* CR pszilagyi: yminsky wanted "argument" indented under the "m" in "some_function".
+   There is a bit of a conflict with how we indent other function call arguments in
+   pattern-matches.  The above is my claim for how to indent this.  What would you say
+   about this one: *)
 let f = fun x -> g
   x
 
 
 
 
+(* dwu *)
 let () =
   f a b ~c:c
     d
   My_module.f a b ~c:c
     d
 
+(* This last case is where Tuareg is inconsistent with the others. *)
 let () =
   My_module.f a b ~c:1.
     d

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

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

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

 
 
 
-(* colon in CR comment *)
+(* colon in CR comment messes Tuareg up *)
 type cfg = {
   foo : int;  (* CR mburns: float? *)
   bar : string;
 }
 
+(* To be more precise about the Tuareg bug, it is the fact that the colon in the comment
+   is the first or second colon after the start of the record definition.  If the comment
+   occurs after the first 2 fields in the record everything is fine.
+
+   For example, this is OK: *)
 type t= {
   foo : int;
   bar : string; (* CR mburns: float? *)
   baz : string;
 }
 
+(* but Tuareg messes this up *)
 type t= {
   foo : int;
   (* CR mburns: float? *)

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

     (G)
     (H)
 
+(* CR pszilagyi: To me, this looks fine as it is.  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 ...
+  let blah _
 end)
   (G)

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

   =
   bar ();
   baz
+
+(* The picture shows where we want the `=' to be.  However, Tuareg currently moves it over
+   to line up with the arguments.
+
+   Perhaps this is merely a personal preference, but that seems ugly to me. *)

File 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'
+
+(* CR pszilagyi: Here we come to a conflict between a couple of indentation principles.  I
+   personally think the immediately above should have "_" under the "f" in "Deferred" and
+   ">>|" under the "D".
+
+   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 right.  I think we favor the first rule. *)

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

       executing_exchange;
       fill_id;
     } -> ()
+(* CR pszilagyi: The principle isn't totally clear in my mind here, but I think: the
+   record is a pattern, so it should be indented at the same level as a simpler pattern,
+   which would be indented 4 right (leaving room for the "|" that would be indented 2
+   right if it were there).  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-syntactic.
+   I suppose ocp-indent already has that problem if its indenting different binary
+   operatorcs differently? *)
 
 let check_price t = function
   | { Exec.

File 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
   h x
 ;;
 
+let _ =
+  (z (fun x -> x)
+   |! Validate.of_list)			(* Tuareg indents this line too far. *)
 
-
-    (z (fun x -> x)
-     |! Validate.of_list)
-
-    (z x
-     |! Validate.of_list)
+let _ =
+  (* Tuareg works correctly on this (if you drop the fun). *)
+  (z x
+   |! Validate.of_list)

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

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

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

     y
 ;;
 
+(* yminsky *)
 let z =
   some_function
     <:sexp_of<foo>>

File 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 *)
-;;
-
-(* -------------------------------------------------------------------------- *)
-
-let f = function
-  | _ -> 0
-;;
-
-let f x = match x with
-  | _ -> 0
-;;
-
-let f =
-  function
-  | _ -> 0 (* This seems to be an anomaly. *)
-;;         (* pszilagyi: "|" used to be indented more; seems right now. *)
-
-let f x =
-  match x with
-  | _ -> 0
-;;
-
-(* -------------------------------------------------------------------------- *)
-
-let handle_query qs ~msg_client:_ = try_with (fun () ->
-  if ... then
-    f >>| fun () ->
-  `Done () (* This is one too far to the left *)
-  else
-    ...
-)
-;;
-
-if ... then ... else
-  assert_branch_has_node branch node >>| fun () ->
-{ t with node; floating; }
-;;
-
-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))
-
-(* -------------------------------------------------------------------------- *)
-
-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 x =
-  { x with
-    foo = 3
-    ; bar = 5                           (* should be 2 left *)
-  }
-
-let x =
-  { (* blah blah blah *)
-    foo = 3
-    ; bar = 5                           (* should be 2 left *)
-  }
-;;
-
-(* -------------------------------------------------------------------------- *)
-
-module M =
-  F (G)
-    (H) (* OK, line up the functor args *)
-
-module M =
-  F
-    (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? *)
-
-(* ------------------------------------------------------------------------ *)
-
-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' (* 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 ->
-                         ...)		(* Should be under "ferred" *)
-                       >>| 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 right.  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. *)
+(* For more, see ../ocaml/contrib/ocp-indent/tests/js-*. *)
 
 (* -------------------------------------------------------------------------- *)
 
   exception E_in
   let () = ()				(* was aligned with "n E_in" *)
 end					(* was aligned with "E_in" *)
-
-(* -------------------------------------------------------------------------- *)
-
-(* sehrlichman *)
-let all_equal =
-  a = b
-  && c = d
-    && e = f (* this && should line up with previous one *)
-;;
-
-(* ereisner: '=' seems to be relevant here *)
-let _ =
-  x
-  && t.entity = entity
-  && t.clearing_firm = clearing_firm
-    && t.type_ = type_
-
-(* -------------------------------------------------------------------------- *)
-
-let _ =
-  [f (fun x ->
-    x);					(* Should be 1 more right. *)
-   f (fun x ->
-     x); (* Why is this out space farther than two lines above? *)
-   f (fun x ->
-     x);
-  ]
-;;
-
-let _ =
-  x
-  >>= fun x ->
-  (try x with _ -> ())
-  >>= fun x ->
-  try x with _ -> ()
-  >>= fun x ->				(* Should be 2 more right. *)
-    x (* Again, why is this farther out? *)
-;;
-
-(* -------------------------------------------------------------------------- *)
-
-let () =
-  foo.bar <-
-    f x
-    y z (* should be two spaces to the right *)
-
-(* yminsky: A small indentation bug I've seen a fair number of times.  Ideally, this: *)
-let should_check_can_sell_and_marking regulatory_regime =
-  match z with
-  | `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
-    argument
-
-(* -------------------------------------------------------------------------- *)
-
-let () =
-  f
-    x
-  <:sexp_of<int>>
-    y
-;;
-
-(* yminsky: This: *)
-let z =
-  some_function
-  <:sexp_of<foo>>                       (* should be indented 2 right *)
-;;
-(* so it works the same as this: *)
-let z =
-  some_function
-    argument
-
-(* -------------------------------------------------------------------------- *)
-
-(* dwu *)
-let () =
-  f a b ~c:c
-    d
-
-let () =
-  f a b ~c:1.
-    d
-
-let () =
-  My_module.f a b ~c:c
-    d
-
-(* This last case is inconsistent with the others - the d is not indented *)
-let () =
-  My_module.f a b ~c:1.
-  d
-
-(* -------------------------------------------------------------------------- *)
-
-(* The following code indents incorrectly: *)
-
-    (z (fun x -> x)
-                 |! Validate.of_list)
-
-(* should be *)
-
-    (z (fun x -> x)
-     |! Validate.of_list)
-
-(* 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: *)
-
-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.: *)
-
-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. *)
-
-(* -------------------------------------------------------------------------- *)
-
-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;
-}
-(* 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 *)
-  a: int;
-}
-
-type t = {
-  (* This is a comment : with a colon. The field lines up to the word after the colon. *)
-                         a: int;
-}
-
-type t = {
-  a: int;
-  (* It also happens with the :     second field *)
-                                    b: int;
-}
-
-type t = {
-  a: int;
-  b: int;
-  (* But strangely not : the third... *)
-  c: int;
-}
-
-(* -------------------------------------------------------------------------- *)
-
-(* colon in CR comment messes this up *)
-type cfg = {
-  foo : int;  (* CR mburns: float? *)
-                            bar : string;
-}
-
-
-(*
-To be more precise about the bug, it is the fact that the colon in
-comment is the first or second colon after the start of the record
-definition.  If the comment occurs after the first 2 fields in the
-record everything is fine.
-
-eg this is ok
-*)
-type t= {
-  foo : int;
-  bar : string; (* CR mburns: float? *)
-  baz : string;
-}
-
-
-(* but this is messed up *)
-
-type t= {
-  foo : int;
- (* CR mburns: float? *)
-               bar : string;
-}