Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MPR#7720, format documentation: maximum indentation limit #1596

Merged
merged 1 commit into from Oct 16, 2018

Conversation

Projects
None yet
4 participants
@Octachron
Copy link
Contributor

commented Feb 3, 2018

This PR proposes to fix a discrepancy between the documentation of Format.set_indent and the implemented behavior of Format on the subject of when a new box is rejected to the left.

The current documentation precises that

pp_set_max_indent ppf d sets the maximum indentation limit of lines to d (in characters): once this limit is reached, new pretty-printing boxes are rejected to the left, if they do not fit on the current line.

With this documentation, it seems natural to expect that

 set_margin 10;
set_max_indent 5; printf "@[123456@[7@]89A@]@."

displays

"123456789A"

since the contents of the box @[7@] does fit on the line.

However, the currently implemented behavior is to reject new box opened after the indentation limit to the left if their parent box does not fit on the line (i.e. if they are not replaced by a fit box). Consequently, the previous example rejects the inner box to the left and prints

123456
789A

whereas both printf "@[123456@[7@]89@]@." and printf "@[123@[456@[7@]89@]A@]@." avoid this rejection to the left and prints 123456789 and 123456789A respectively.

This PR updates the documentation to reflect the current behavior and adds the three examples above to illustrate this behavior.

@Octachron Octachron requested a review from pierreweis Feb 3, 2018

@pierreweis

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2018

Thanks for tracking this documentation bug. In fact, the correct phrasing is rather involving. The fact that the new box is rejected to the left depends on the size AND nature of the englobing box. If the englobing box is horizontal or fully fits on the line then the new box is not rejected on the left.
Given that an horizontal box is always considered fitting on the current line, and a vertical box is always not fitting, we can rephrase the documentation as:

... rejected to the left, unless the englobing box fully fits on the current line.

@rbonichon will shortly add examples to show the full range of behaviors

@rbonichon

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2018

The toplevel session below shows in more detail the expected behavior with some types of boxes, and some further comments:

  • The b-, hv- and hov- boxes are all compacting and will try to fit as much as possible on the same line. That's why on the run below, the behave the same on the example.
  • v-boxes are not : what counts in this case is only where you open the inner box w.r.t to max_indent.
  • the behavior of the line break is somewhat akin to having a break hint automatically included right before the inner box in the case of compacting boxes only. As you can see below in "@[<v 1>12345@,@[6@]789AAA@]@." , the explicit break hint for the v-box does break the line, as expected, whereas, the version without it has no line break.
open Format ;;

set_margin 10 ;;
set_max_indent 5 ;;

let g s = Scanf.format_from_string s "" ;;

let t s =
  printf "** %s@." s;
  let fmt = g s in printf fmt ;
;;

t "@[123456@[7@]89A@]@." ;;

t "@[123456@[7@]89@]@." ;;

t "@[<hv>123456@[7@]89A@]@." ;;

t "@[<hv>123456@[7@]89@]@." ;;

t "@[<v 1>123456@[7@]89@]@." ;;

t "@[<v 1>12345@[6@]789AAA@]@." ;;

(* with explicit breaks *)
t "@[123456@,@[7@]89A@]@." ;;

t "@[123456@,@[7@]89@]@." ;;

t "@[<hv>123456@,@[7@]89A@]@." ;;

t "@[<hv>123456@,@[7@]89@]@." ;;

t "@[<v 1>123456@,@[7@]89@]@." ;;

t "@[<v 1>12345@,@[6@]789AAA@]@." ;;

(** Evaluation session
# t "@[123456@[7@]89A@]@.";;
** @[123456@[7@]89A@]@.
123456
789A
- : unit
= ()
# t "@[123456@[7@]89@]@.";;
** @[123456@[7@]89@]@.
123456789
- : unit
= ()
# t "@[<v 1>123456@[7@]89@]@.";;
** @[<v 1>123456@[7@]89@]@.
123456
 789
- : unit
= ()
# t "@[<v 1>12345@[6@]789AAA@]@.";;
** @[<v 1>12345@[6@]789AAA@]@.
123456789AAA
- : unit
= ()
# t "@[<hv>123456@[7@]89A@]@.";;
** @[<hv>123456@[7@]89A@]@.
123456
789A
- : unit
= ()
# t "@[<hv>123456@[7@]89@]@.";;
** @[<hv>123456@[7@]89@]@.
123456789
- : unit
= ()
# t "@[123456@,@[7@]89A@]@.";;
** @[123456@,@[7@]89A@]@.
123456
789A
- : unit
= ()
# t "@[123456@,@[7@]89@]@.";;
** @[123456@,@[7@]89@]@.
123456789
- : unit
= ()
# t "@[<hv>123456@,@[7@]89A@]@.";;
** @[<hv>123456@,@[7@]89A@]@.
123456
789A
- : unit
= ()
# t "@[<hv>123456@,@[7@]89@]@.";;
** @[<hv>123456@,@[7@]89@]@.
123456789
- : unit
= ()
# t "@[<v 1>123456@,@[7@]89@]@.";;
** @[<v 1>123456@,@[7@]89@]@.
123456
 789
- : unit
= ()
# t "@[<v 1>12345@,@[6@]789AAA@]@.";;
** @[<v 1>12345@,@[6@]789AAA@]@.
12345
 6789AAA
- : unit
= ()
 *)
@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Feb 27, 2018

@pierreweis , @rbonichon Thanks for the review!

I have amended the wording on the core explanation as suggested by @pierreweis , replacing englobing by enclosing because it sounded very french to my ear.

I have also added a small note on the fact that vertical and horizontal boxes respectively never and always fit.

I am not sure which examples proposed by @rbonichon I should add to the documentation, entering in too much detail sounds like a good way to lose the reader at this point. I have the impression that the current 3 examples covers the core behavior, is there any important point that is missing?

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2018

@rbonichon , @pierreweis : is there something amiss with the documentation added by this PR?

or opening an intermediary box before the maximum indentation limit which
fits on the current line
{[ printf "@[123@[456@[7@]89@]A@]@." ]}
avoid the rejection to the left of the inner boxes and print respectively

This comment has been minimized.

Copy link
@rbonichon

rbonichon Sep 3, 2018

Contributor

x or y avoid -> x or y avoids (I think)

This comment has been minimized.

Copy link
@Octachron

Octachron Sep 3, 2018

Author Contributor

Indeed.

]}
because the nested box ["@[7@]"] is opened after the maximum indentation
limit ([7>5]) and its parent box does not fit on the current line.
Contrarily, either decreasing the length of the parent box to make it fit

This comment has been minimized.

Copy link
@rbonichon

rbonichon Sep 3, 2018

Contributor

Not sure about "Contrarily" here (I do not feel a big opposition with what was said before). Maybe we could just go with "Either" (I agree I am nitpicking)

This comment has been minimized.

Copy link
@Octachron

Octachron Sep 3, 2018

Author Contributor

Good point, I removed the contrarily.

@rbonichon

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2018

@Octachron Ok for me (Left 2 very minor comments)

@Octachron Octachron force-pushed the Octachron:format_doc_max_indent branch from 16d6c53 to adf4c4c Sep 3, 2018

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2018

Thanks a lot for the comments!

@gasche gasche merged commit c48371c into ocaml:trunk Oct 16, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.