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

Ability to include and customize "Trailing separators" #2

Open
jordwalke opened this issue Dec 31, 2014 · 7 comments
Open

Ability to include and customize "Trailing separators" #2

jordwalke opened this issue Dec 31, 2014 · 7 comments

Comments

@jordwalke
Copy link
Collaborator

It would be great to have a few features for specifying if separators should be included in the "trailing" portion of the list.

A couple of options:

``Trailing_separator_never`

// Self explanatory

``Trailing_separator_always`

{key: x, key2: y,}        // If not having to break

{                                // If having to break
  key: x,
  key2: y,
}

And

``Trailing_separator_if_breaks`

{key: x, key2: y}           // If not having to break

{                                    // If having to break
  key: x,
  key2: y,
}

You mentioned that Trailing_separator_always could be accomplished via wrapping in an Atom. I will try to confirm that. But I'm not sure how I would accomplish Trailing_separator_if_breaks.

@mjambon
Copy link
Member

mjambon commented Nov 28, 2015

I don't know how we can achieve Trailing_separator_if_breaks. The Format` module provides this:

val print_if_newline : unit -> unit

"Executes the next formatting command if the preceding line has just been split. Otherwise, ignore the next formatting command."

We'd like to print something before the decision to print a newline is made. It would be problematic if what we want to print is too long.

@jordwalke
Copy link
Collaborator Author

Yes, I agree. Your concern is that the decision to break doesn't take into account the length of the separator itself? You could have a clarification that "print trailing separator only if break" might make line length maximums more of a "target" than a "maximum". It seems much better than whatever hacks I would come up with otherwise.

@jordwalke
Copy link
Collaborator Author

For posterity, I thought I'd add one more use case here: Variant types: You may only want to include the leading bar if the line breaks.

type suit = Spades | Diamonds | Hearts | Clubs
type suit =
  | Spades
  | Diamonds
  | Hearts
  | Clubs

jordwalke referenced this issue in reasonml/reason Nov 28, 2015
Summary:Per request of @jberdine, we no longer require the final
semicolon. There's a small tradeoff that had to be made (no punned,
single item records) - but that case is very rare, and would surely be
caught by the type system.

I also took the opportunity to start a clean directory of type checked
formatting test cases. Many bugs in parsing/printing would be caught by
simple type checking of the AST.

Thanks for the suggesion, Josh. I like this much better.

The next step is to make it so when pretty printing, the final semicolon
is ommitted.

Similarly, it is easy to make it such that the final item in a
module/signature does not require a semicolon.

Test Plan:Added type-checked formatting tests.

Reviewers:@jberdine, @cristianoc

CC:
@jberdine
Copy link

Am I overlooking something, or is the situation for printing leading separators only after line breaks possible using print_if_newline?

Would it be possible to hack up an implementation of Trailing_separator_if_breaks by using Format. set_formatter_out_functions? After opening the box containing a List, the out_newline function could be set to one that sets a bool ref (associated with the List) and then acts as the default. Then when printing the final separator, the bool ref could be queried to determine if a line break has occurred while formatting the List, and if so to print the separator, otherwise don't print it. I wonder if there are nasty cases involving unflushed format queues that have not yet called out_newline when formatting the final separator queries the state. Perhaps executing Format.pp_print_string on the empty string (in order to trigger Format.advance_left), and decreasing the margin by the length of the closing string would reduce or eliminate these.

@mjambon
Copy link
Member

mjambon commented Nov 29, 2015

@jordwalke @jberdine I only mentioned Format.print_if_newline because it somewhat resembles a function that we would need. I don't see how it would solve our problem, though.

@mjambon
Copy link
Member

mjambon commented Nov 29, 2015

@jberdine Format.set_formatter_out_functions is interesting. I haven't experimented in that direction yet. Looking at the code for format.ml could help, too. I too am worried about when the formatter_out_functions are called.

mjambon referenced this issue Dec 13, 2015
Force breaks after labels if child requires so
@yunxing
Copy link

yunxing commented Jan 1, 2017

Looking into this problem. Another thing maybe we can do is to temporarily overwrite the "out_newline" function only when we are printing the last item of a list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants