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

Introduce semi-expanded format for all containers #309

Merged
merged 5 commits into from
Jun 23, 2021

Conversation

michalmuskala
Copy link
Member

This change implements the semi-expanded format introduced for function calls in #305 to all the containers.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 23, 2021
LastFitsFun = last_fits_fun(LastFits, Values),
BreakKind = break_behaviour(Meta, Values, BreakKind0),
BreakFun = break_fun(BreakKind),
BreakBehaviour = break_behaviour(Meta, Values, BreakKind),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice naming

@@ -535,16 +527,14 @@ has_opening_line_break(Meta, [HeadValue | _]) ->

has_any_break_between([Head | [Head2 | _] = Tail]) ->
has_break_between(Head, Head2) orelse has_any_break_between(Tail);
has_any_break_between([{cons, _, Head, Tail}]) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see this is for the new case you brought up

group(concat(nest(concat(Left, Doc), ?INDENT, break), Right)).
group(concat(nest(concat(Left, Doc), ?INDENT, break), Right));
surround_container(#break{inner = ForceInner, outer = ForceOuter}, Left, Doc, Right) ->
InnerDoc = group(concat(maybe_force_breaks(ForceInner), Doc)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So Doc is now always grouped, does this still cover the line case, which was not grouped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, using force_breaks() turns break() into line() so that still works correctly and removes one case

),
?assertSame(
"-export([\n"
" bar/1, baz/1\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting.
I guess this is now a thing.
Almost feel like export should be special cased, but it is probably fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I thought about this as well, but it seems consistent, if everything fits on a single line

"-export([\n"
" foo/1,\n"
" bar/1,\n"
" baz/1, baz/2\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great test

@michalmuskala michalmuskala merged commit f109b8c into master Jun 23, 2021
@michalmuskala michalmuskala deleted the all-semi-expanded branch June 23, 2021 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants