Skip to content

Commit

Permalink
Merge pull request #10 from mjambon/mj/break-labels
Browse files Browse the repository at this point in the history
Force breaks after labels if child requires so
  • Loading branch information
mjambon committed Dec 7, 2015
2 parents e7376cf + 07c5588 commit 6e0b27b
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 30 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@ nativecode
lambda_example
simple_example
test_easy_format

easy_format_example.html
easy_format_example.ml
ocamldoc
62 changes: 47 additions & 15 deletions easy_format.ml
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
open Format

type wrap =
[ `Wrap_atoms
| `Always_wrap
| `Never_wrap
| `Force_breaks
| `Force_breaks_rec
| `No_breaks ]
type wrap = [
| `Wrap_atoms
| `Always_wrap
| `Never_wrap
| `Force_breaks
| `Force_breaks_rec
| `No_breaks
]

type label_break = [
| `Auto
| `Always
| `Always_rec
| `Never
]

type style_name = string
type style = {
Expand Down Expand Up @@ -57,12 +65,14 @@ let list = {
}

type label_param = {
label_break: label_break;
space_after_label : bool;
indent_after_label : int;
label_style : style_name option;
}

let label = {
label_break = `Auto;
space_after_label = true;
indent_after_label = 2;
label_style = None;
Expand Down Expand Up @@ -118,12 +128,13 @@ let propagate_from_leaf_to_root
has the attribute wrap_body = `Force_breaks_rec.
*)
let propagate_forced_breaks x =
(* acc = whether to force breaks in wrappable lists *)
(* acc = whether to force breaks in wrappable lists or labels *)
let init_acc = function
| List ((_, _, _, { wrap_body = `Force_breaks_rec }), _)
| Label ((_, { label_break = `Always_rec }), _) -> true
| Atom _
| Label _
| Custom _ -> false
| List ((_, _, _, { wrap_body = `Force_breaks_rec }), _) -> true
| Custom _
| List _ -> false
in
let merge_acc force_breaks1 force_breaks2 =
Expand All @@ -144,9 +155,16 @@ let propagate_forced_breaks x =
else
x, false

| Label ((a, ({ label_break = `Auto } as lp)), b) ->

This comment has been minimized.

Copy link
@jordwalke

jordwalke Dec 12, 2015

Collaborator

I tested this out and it works pretty well. The one thing I might suggest is:

  1. Labels that have a child with Force_breaks_rec or Always_rec in either their "left" or "right" component, should propagate upwards as they do.
  2. However, the label itself should likely only be forced broken if the right component has some propagated breakage.

I'll add a test case to explain why I think this is the right convention, and I think I know how I would implement this.

This comment has been minimized.

Copy link
@mjambon

mjambon Dec 13, 2015

Author Member

I think I understand rule (2). The right component of the label is really a child of the left component and their relationship should not be affected by the other descendants of the left component. An example would be good, still.

This comment has been minimized.

Copy link
@jordwalke

jordwalke Dec 13, 2015

Collaborator

Yes, exactly.

if force_breaks then
let lp = { lp with label_break = `Always } in
Label ((a, lp), b), true
else
x, false

| List ((_, _, _, { wrap_body = `No_breaks }), _)
| Label ((_, { label_break = (`Always | `Always_rec | `Never) }), _)
| Atom _
| Label _
| Custom _ -> x, force_breaks
in
let new_x, forced_breaks =
Expand Down Expand Up @@ -492,10 +510,22 @@ struct
fprint_t fmt lab;
close_tag fmt lp.label_style;

if lp.space_after_label then
pp_print_break fmt 1 indent
else
pp_print_break fmt 0 indent;
(match lp.label_break with
| `Auto ->
if lp.space_after_label then
pp_print_break fmt 1 indent
else
pp_print_break fmt 0 indent
| `Always
| `Always_rec ->
pp_force_newline fmt ();
pp_print_string fmt (String.make indent ' ')
| `Never ->
if lp.space_after_label then
pp_print_char fmt ' '
else
()
);
fprint_t fmt x;
pp_close_box fmt ()

Expand Down Expand Up @@ -620,12 +650,14 @@ struct
}

let label_true = {
label_break = `Auto;
space_after_label = true;
indent_after_label = 2;
label_style = None;
}

let label_false = {
label_break = `Auto;
space_after_label = false;
indent_after_label = 2;
label_style = None;
Expand Down
37 changes: 29 additions & 8 deletions easy_format.mli
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ type wrap =
i.e. never break line between list items
*)

type label_break = [
| `Auto
| `Always
| `Always_rec
| `Never
]
(** When to break the line after a [Label]:
- [Auto]: break after the label if there's not enough room
- [Always]: always break after the label
- [Always_rec]: always break after the label and force breaks in all parent
lists and labels, similarly to [`Force_breaks_rec] for lists.
- [Never]: never break after the label
*)

type style_name = string

Expand Down Expand Up @@ -116,14 +129,22 @@ val list : list_param
See {!Easy_format.label}.
*)
type label_param = {
space_after_label : bool; (** Whether there must be some whitespace
after the label.
Default: [true] *)
indent_after_label : int; (** Extra indentation before the item
that comes after a label.
Default: [2]
*)
label_style : style_name option; (** Default: [None] *)
label_break: label_break;
(** Whether to break the line after the label.
Introduced in version 1.2.0.
Default: [`Auto] *)

space_after_label : bool;
(** Whether there must be some whitespace after the label.
Default: [true] *)

indent_after_label : int;
(** Extra indentation before the item that comes after a label.
Default: [2]
*)

label_style : style_name option;
(** Default: [None] *)
}

val label : label_param
Expand Down
13 changes: 7 additions & 6 deletions simple_example.ml
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,18 @@ let format_function_definition (body_label, body_param) name param body =
Illustrate the difference between `Force_break and `Force_breaks_rec on
labels.
*)
let labelOneAtom = Atom ("reallyLongLabelOne", atom)
let labelTwoAtom = Atom ("reallyLongLabelTwo", atom)
let labelThreeAtom = Atom ("reallyLongLabelABC", atom)
let label_one_atom = Atom ("reallyLongLabelOne", atom)
let label_two_atom = Atom ("reallyLongLabelTwo", atom)
let label_three_atom = Atom ("reallyLongLabelABC", atom)
let make_list_in_labels (wrap) =
Label (
(labelOneAtom, label),
(label_one_atom, label),
(
Label (
(labelTwoAtom, label),
(label_two_atom, label),
(
Label (
(labelThreeAtom, label),
(label_three_atom, label),
List (
("[", ",", "]", { list with wrap_body = wrap }),
[
Expand All @@ -152,6 +152,7 @@ let make_list_in_labels (wrap) =
)
)
)

(*
Illustrate the difference between `Force_break and `Force_breaks_rec
*)
Expand Down

0 comments on commit 6e0b27b

Please sign in to comment.