Add support for preserving empty lines (#31)

* First, very opinionated, version

* Add support for preserving empty lines

README.md

@@ -54,6 +54,14 @@ The plugin supports the following configuration options in the `format` section
     - Erlang's `prettypr` inserts a tab character each time it has to insert 8 spaces for indentation and that code is in a 100% unconfigurable/unreplaceable/unhookable function. If this setting is `true`, the formatter will turn those tabs into 8 spaces again.
     - The default value is `true`.
     - **NOTE:** We are aware that `true` is not the _actual OTP default_ but... **really?** Who wants their code indented with a mixture of tabs and spaces? 🙈
+* `inline_expressions` (`boolean()`):
+    - Specifies if sequential expressions in a clause should be placed in the same line if `paper` and `ribbon` allows it or if each expression should be placed in its own line.
+    - The default value is `true`.
+* `preserve_empty_lines` (`boolean()`):
+    - Specifies if blank lines should be preserved when formatting.
+    - This option is only used when `inline_expressions` is `false`.
+    - If this option is `true`, one empty line will preserved for each group of empty lines that are placed between expressions in a clause.
+    - The default value is `false`.
 
 ### Per-File Configuration
 

elvis.config

@@ -2,6 +2,5 @@
     dirs => ["src"],
     src_dirs => ["src"],
     filter => "*.erl",
-    ruleset => erl_files,
-    ignore => [rebar3_prettypr] %% Copied from OTP, doesn't yet follow our rules
+    ruleset => erl_files
 }}]}].

src/rebar3_formatter.erl

@@ -6,7 +6,8 @@
 -type opts() :: #{files => [file:filename_all()],
                   output_dir => undefined | string(), encoding => none | epp:source_encoding(),
                   paper => pos_integer(), ribbon => pos_integer(), break_indent => pos_integer(),
-                  sub_indent => pos_integer(), remove_tabs => boolean()}.
+                  sub_indent => pos_integer(), remove_tabs => boolean(),
+                  inline_expressions => boolean(), preserve_empty_lines => boolean()}.
 
 -export_type([opts/0]).
 
@@ -46,17 +47,22 @@ format(File, AST, Comments, Opts) ->
     BreakIndent = maps:get(break_indent, Opts, 4),
     SubIndent = maps:get(sub_indent, Opts, 2),
     RemoveTabs = maps:get(remove_tabs, Opts, true),
+    InlineExpressions = maps:get(inline_expressions, Opts, true),
+    PreserveEmptyLines = maps:get(preserve_empty_lines, Opts, false),
     FinalFile = case maps:get(output_dir, Opts) of
                   undefined -> File;
                   OutputDir -> filename:join(filename:absname(OutputDir), File)
                 end,
     ok = filelib:ensure_dir(FinalFile),
     FormatOpts = [{paper, Paper}, {ribbon, Ribbon}, {encoding, Encoding},
-                  {break_indent, BreakIndent}, {sub_indent, SubIndent}],
+                  {break_indent, BreakIndent}, {sub_indent, SubIndent},
+                  {inline_expressions, InlineExpressions}],
     ExtendedAST = AST ++ [{eof, 0}],
     WithComments = erl_recomment:recomment_forms(erl_syntax:form_list(ExtendedAST),
                                                  Comments),
-    PreFormatted = rebar3_prettypr:format(WithComments, FormatOpts),
+    PreFormatted = rebar3_prettypr:format(WithComments,
+                                          empty_lines(InlineExpressions, PreserveEmptyLines, File),
+                                          FormatOpts),
     Formatted = maybe_remove_tabs(RemoveTabs,
                                   unicode:characters_to_binary(PreFormatted, Encoding)),
     file:write_file(FinalFile, Formatted).
@@ -65,3 +71,18 @@ maybe_remove_tabs(false, Formatted) -> Formatted;
 maybe_remove_tabs(true, Formatted) ->
     binary:replace(Formatted, <<"\t">>, <<"        ">>, [global]).
 
+empty_lines(true, _, _) -> [];
+empty_lines(false, false, _) -> [];
+empty_lines(false, true, File) ->
+    {ok, Data} = file:read_file(File),
+    List = binary:split(Data, [<<"\n">>], [global, trim]),
+    {ok, NonEmptyLineRe} = re:compile("\\S"),
+    {Res, _} = lists:foldl(fun (Line, {EmptyLines, N}) ->
+                                   case re:run(Line, NonEmptyLineRe) of
+                                     {match, _} -> {EmptyLines, N + 1};
+                                     nomatch -> {[N | EmptyLines], N + 1}
+                                   end
+                           end,
+                           {[], 1}, List),
+    lists:reverse(Res).
+

src/rebar3_prettypr.erl

@@ -10,7 +10,7 @@
 
 -module(rebar3_prettypr).
 
--export([format/2]).
+-export([format/3]).
 
 -import(prettypr,
         [text/1, nest/2, above/2, beside/2, sep/1, par/1, par/2, floating/3, floating/1,
@@ -40,6 +40,7 @@
          break_indent = ?BREAK_INDENT  :: non_neg_integer(),
          clause = undefined  :: clause_t() | undefined, paper = ?PAPER  :: integer(),
          ribbon = ?RIBBON  :: integer(), user = ?NOUSER  :: term(),
+         inline_expressions = true  :: boolean(), empty_lines = []  :: [pos_integer()],
          encoding = epp:default_encoding()  :: epp:source_encoding()}).
 
 set_prec(Ctxt, Prec) ->
@@ -49,20 +50,13 @@ reset_prec(Ctxt) ->
     set_prec(Ctxt, 0).    % used internally
 
 %% =====================================================================
-%% @spec format(Tree::syntaxTree(), Options::[term()]) -> string()
-%%
-%% @type syntaxTree() = erl_syntax:syntaxTree().
-%%
-%% An abstract syntax tree. See the {@link erl_syntax} module for
-%% details.
-%%
 %% @doc Prettyprint-formats an abstract Erlang syntax tree as text. For
 %% example, if you have a `.beam' file that has been compiled with
 %% `debug_info', the following should print the source code for the
 %% module (as it looks in the debug info representation):
 %% ```{ok,{_,[{abstract_code,{_,AC}}]}} =
 %%            beam_lib:chunks("myfile.beam",[abstract_code]),
-%%    io:put_chars(rebar3_prettypr:format(erl_syntax:form_list(AC)))
+%%    io:put_chars(rebar3_prettypr:format(erl_syntax:form_list(AC), [], []))
 %% '''
 %%
 %% Available options:
@@ -83,6 +77,11 @@ reset_prec(Ctxt) ->
 %%       <dd>Specifies the number of spaces to use for breaking indentation.
 %%       The default value is 2.</dd>
 %%
+%%   <dt>{inline_expressions, boolean()}</dt>
+%%       <dd>Specifies wether multiple sequential expressions within the
+%%       same clause can be placed in the same line (if paper/ribbon permits).
+%%       The default value is true.</dd>
+%%
 %%   <dt>{encoding, epp:source_encoding()}</dt>
 %%       <dd>Specifies the encoding of the generated file.</dd>
 %% </dl>
@@ -91,12 +90,12 @@ reset_prec(Ctxt) ->
 %% @see format/1
 %% @see layout/2
 
--spec format(erl_syntax:syntaxTree(), [term()]) -> string().
+-spec format(erl_syntax:syntaxTree(), [pos_integer()], [term()]) -> string().
 
-format(Node, Options) ->
+format(Node, EmptyLines, Options) ->
     W = proplists:get_value(paper, Options, ?PAPER),
     L = proplists:get_value(ribbon, Options, ?RIBBON),
-    prettypr:format(layout(Node, Options), W, L).
+    prettypr:format(layout(Node, EmptyLines, Options), W, L).
 
 %% =====================================================================
 %% @spec layout(Tree::syntaxTree(), Options::[term()]) -> prettypr:document()
@@ -116,14 +115,17 @@ format(Node, Options) ->
 %% @see prettypr
 %% @see format/2
 
--spec layout(erl_syntax:syntaxTree(), [term()]) -> prettypr:document().
+-spec layout(erl_syntax:syntaxTree(), [pos_integer()],
+             [term()]) -> prettypr:document().
 
-layout(Node, Options) ->
+layout(Node, EmptyLines, Options) ->
     lay(Node,
         #ctxt{paper = proplists:get_value(paper, Options, ?PAPER),
               ribbon = proplists:get_value(ribbon, Options, ?RIBBON),
               break_indent = proplists:get_value(break_indent, Options, ?BREAK_INDENT),
               sub_indent = proplists:get_value(sub_indent, Options, ?SUB_INDENT),
+              inline_expressions = proplists:get_value(inline_expressions, Options, true),
+              empty_lines = EmptyLines,
               encoding = proplists:get_value(encoding, Options, epp:default_encoding())}).
 
 lay(Node, Ctxt) ->
@@ -254,8 +256,7 @@ lay_no_comments(Node, Ctxt) ->
                  none -> none;
                  G -> lay(G, Ctxt1)
                end,
-          D3 = sep(seq(erl_syntax:clause_body(Node), lay_text_float(","), Ctxt1,
-                       fun lay/2)),
+          D3 = lay_clause_expressions(erl_syntax:clause_body(Node), Ctxt1, fun lay/2),
           case Ctxt#ctxt.clause of
             fun_expr -> make_fun_clause(D1, D2, D3, Ctxt);
             {function, N} -> make_fun_clause(N, D1, D2, D3, Ctxt);
@@ -922,5 +923,23 @@ tidy_float_second([$e | Cs]) -> tidy_float_second([$e, $+ | Cs]);
 tidy_float_second([_C | Cs]) -> tidy_float_second(Cs);
 tidy_float_second([]) -> [].
 
+lay_clause_expressions(Exprs, Ctxt = #ctxt{inline_expressions = true}, Fun) ->
+    sep(seq(Exprs, floating(text(",")), Ctxt, Fun));
+lay_clause_expressions([H], Ctxt, Fun) -> Fun(H, Ctxt);
+lay_clause_expressions([H | T], Ctxt, Fun) ->
+    Clause = beside(Fun(H, Ctxt), floating(text(","))),
+    Next = lay_clause_expressions(T, Ctxt, Fun),
+    case is_last_and_before_empty_line(H, T, Ctxt) of
+      true -> above(above(Clause, text("")), Next);
+      false -> above(Clause, Next)
+    end;
+lay_clause_expressions([], _, _) -> empty().
+
+is_last_and_before_empty_line(H, [], #ctxt{empty_lines = EmptyLines}) ->
+    lists:member(erl_syntax:get_pos(H) + 1, EmptyLines);
+is_last_and_before_empty_line(H, [H2 | _], #ctxt{empty_lines = EmptyLines}) ->
+    erl_syntax:get_pos(H2) - erl_syntax:get_pos(H) >= 2 andalso
+      lists:member(erl_syntax:get_pos(H) + 1, EmptyLines).
+
 
 %% =====================================================================

test_app/after/src/empty_lines.erl

@@ -0,0 +1,30 @@
+-module(empty_lines).
+
+-compile(export_all).
+
+-format([{inline_expressions, false}, {preserve_empty_lines, true}]).
+
+in_this() ->
+    function,
+    there:are(no, empty, lines).
+
+this_function() ->
+    has:two(),
+
+    empty:lines(),
+    first:one(wont, be, preserved).
+
+here() ->
+    we:have(many),
+
+    empty:lines(),
+
+    but:only(two, should),
+    be:preserved().
+
+empty(Lines) ->
+    should:nt(be, preserved, if_they:appear(within, a:single(expression))),
+    but:we_preserve(Lines),
+
+    between:them().
+

test_app/after/src/inline_expressions.erl

@@ -0,0 +1,28 @@
+-module(inline_expressions).
+
+-compile(export_all).
+
+-format([{inline_expressions, false}]).
+
+these() ->
+    Expressions = should:occupy(),
+    a,
+    line,
+    each;
+these() ->
+    other,
+    expressions,
+    too.
+
+also() ->
+    these,
+    two.
+
+even() ->
+    when_they:are(small),
+    enough:to(fit).
+
+white() ->
+    lines:should(not be:preserved()),
+    Since = preserve_empty_lines:is(false).
+

test_app/src/empty_lines.erl

@@ -0,0 +1,38 @@
+-module(empty_lines).
+
+-compile(export_all).
+
+-format([{inline_expressions, false}, {preserve_empty_lines, true}]).
+
+in_this() -> function, there:are(no, empty, lines).
+
+this_function() ->
+
+    has:two(),
+
+    empty:lines(), first:one(wont, be, preserved).
+
+here() ->
+    we:have(many),
+
+
+
+
+    empty:lines(),
+
+
+    but:only(two, should),
+    be:preserved().
+
+empty(Lines) ->
+    should:nt(
+        be,
+
+        preserved, if_they:appear(
+            within,
+
+            a:single(expression))),
+
+    but:we_preserve(Lines),
+
+    between:them().

test_app/src/inline_expressions.erl

@@ -0,0 +1,20 @@
+-module(inline_expressions).
+
+-compile(export_all).
+
+-format([{inline_expressions, false}]).
+
+these() -> Expressions = should:occupy(), a, line, each;
+
+these() ->
+    other,
+    expressions,
+    too.
+
+also() -> these, two.
+even() -> when_they:are(small), enough:to(fit).
+
+white() ->
+    lines:should(not be:preserved()),
+
+    Since = preserve_empty_lines:is(false).