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

Preservation of empty lines is broken #83

Closed
elbrujohalcon opened this issue Feb 26, 2020 · 3 comments
Closed

Preservation of empty lines is broken #83

elbrujohalcon opened this issue Feb 26, 2020 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@elbrujohalcon
Copy link
Collaborator

A file like this one…

-module(f_bench).

-compile(export_all).

a_fun(With, Some, Arguments) ->
    an:expression(that,
                  occupies,
                  Some,
                  #{lines => since, it => uses, a => [long, list], 'of' => Arguments}),


    %% a couple of empty lines below the aforementioned long expression
    another:expression(With, smaller, size),

    %% an empty line that is preserved
    ok.

…results in…

-module(f_bench).

-compile(export_all).

a_fun(With, Some, Arguments) ->
    an:expression(that,
                  occupies,
                  Some,
                  #{lines => since, it => uses, a => [long, list], 'of' => Arguments}),
    %% a couple of empty lines below the aforementioned long expression
    another:expression(With, smaller, size),

    %% an empty line that is preserved
    ok.

…while it should result in…

-module(f_bench).

-compile(export_all).

a_fun(With, Some, Arguments) ->
    an:expression(that,
                  occupies,
                  Some,
                  #{lines => since, it => uses, a => [long, list], 'of' => Arguments}),

    %% a couple of empty lines below the aforementioned long expression
    another:expression(With, smaller, size),

    %% an empty line that is preserved
    ok.
@elbrujohalcon elbrujohalcon added the bug Something isn't working label Feb 26, 2020
@elbrujohalcon
Copy link
Collaborator Author

The issue is in the second clause of default_formatter:is_last_and_before_empty_line/2:

is_last_and_before_empty_line(H, [], #ctxt{empty_lines = EmptyLines}) ->
    lists:member(get_pos(H) + 1, EmptyLines);
is_last_and_before_empty_line(H, [H2 | _], #ctxt{empty_lines = EmptyLines}) ->
    get_pos(H2) - get_pos(H) >= 2 andalso lists:member(get_pos(H) + 1, EmptyLines).

get_pos(H) + 1 may not be empty, because it might be part of H if H uses more than one line to be printed. Maybe we should use get_pos(H2) - 1 instead… or maybe figure out if any of the lines between get_pos(H2) and get_pos(H) is empty.

@elbrujohalcon
Copy link
Collaborator Author

This was not completely fixed, see https://github.com/AdRoll/erliam/pull/5/files#r409081479

@elbrujohalcon elbrujohalcon reopened this Apr 16, 2020
@elbrujohalcon elbrujohalcon added this to the 0.3.0 milestone Apr 16, 2020
elbrujohalcon pushed a commit to AdRoll/erliam that referenced this issue Apr 16, 2020
elbrujohalcon pushed a commit to AdRoll/erliam that referenced this issue Apr 16, 2020
* Add rebar3 formatting

* Remove old way

* Remove lint warnings

* Actually verify the formatting on CI

* Remove support for OTP 20

* Revert indentation of long lists

* Moving comments until AdRoll/rebar3_format#83 is fixed
@elbrujohalcon
Copy link
Collaborator Author

elbrujohalcon commented Apr 28, 2020

Actually… that thing is by design. The formatter does not preserve empty spacing between clauses. It doesn't preserve it between function clauses, case clauses, if clauses, etc…
Only between statements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant