Comments: Field comment moved outside declaration

[richcarl]

Original:

-record(state, {callback,    % callback module
                pass = 0,
                fail = 0,
                skip = 0,
                cancel = 0,
                state        % substate
               }).

Formatted:

% callback module
-record(state, {
    callback,
    pass = 0,
    fail = 0,
    skip = 0,
    cancel = 0,
    % substate
    state
}).

[michalmuskala]

I understand this is not expected in here, but the behaviour is consistent with moving the trailing comment to the line before.

There are places, I've seen in the wild, where this is the desired behaviour. For example:

apply(Module, Fun,     % dynamic call
      build_arguments(State)).

Formatting this as:

% dynamic call
apply(
    Module, 
    Fun,     
    build_arguments(State)
).

seems more correct than

apply(
    Module, 
    % dynamic call
    Fun,
    build_arguments(State)
).

even if we did floating comments, the issue wouldn't be solved since:

apply(
    Module, 
    Fun, % dynamic call
    build_arguments(State)
).

would be equally incorrect.

It seems it's impossible to differentiate.

[richcarl]

It's certainly impossible to know (for a machine) whether the comment pertains to the nearest item or to the item at the start of the line (or any inbetween). Personally, I'd prefer to punish people who use the latter style (so erl_prettypr always attaches to the nearest item).

[elbrujohalcon]

You can always punish nobody

Sorry, @michalmuskala … I couldn't resist. 🙄

Jokes aside, that's exactly why we wrote rebar3_format in the way we wrote it and why we allow users to use erlfmt as a formatter there… Nobody needs to be punished and everyone can read the same code but with the comments placed wherever they like them the most.

[michalmuskala]

It's not about preference, though. In both cases one format is correct and the other is incorrect, the choice only changes which case is correct.

Additionally the formatting roundtrip though a different formatter wouldn't result in getting the same code back, e.g. original -> erlfmt -> rebar3_format default would produce something like:

% callback module
-record(state, {callback,
                pass = 0,
                fail = 0,
                skip = 0,
                cancel = 0,
                % substate
                state
}).

I'm not sure how the choice of formatter helps here.

[elbrujohalcon]

Oh, right! Sorry, I jumped over too quickly.
Since this is related to comments (i.e. outside the scope of parsing and rendering the AST), you're entirely correct: once a comment is moved by a formatter, it won't come back to its original location.
Disregard my previous comment.

[alanz]

Given the line is expanded while formatting anyway, would it not make sense to turn it into

-record(state, {
    % callback module
    callback,
    pass = 0,
    fail = 0,
    skip = 0,
    cancel = 0,
    % substate
    state
}).

[michalmuskala]

It would make sense in this case, yes. It would unfortunately be worse than current formatting for other cases that I shown in #134 (comment)

[awalterschulze]

Just adding a data point. I was looking at @rvirding 's luerl and found that the style that richard referenced in this issue is quite popular there https://github.com/rvirding/luerl/pull/133/files?file-filters%5B%5D=.hrl#diff-526cd0f79036f27ed96d792ce69e68f8L24

[awalterschulze]

Another data point https://github.com/erlang/otp/pull/2756/files/f3c7a3350952b7d0786d9c5baa5d2c9b8805aa9b#diff-1315c36aca592e79ef0073fcf3019e5fR282

[awalterschulze]

Another data point https://github.com/erlang/otp/pull/2756/files/f3c7a3350952b7d0786d9c5baa5d2c9b8805aa9b#diff-1315c36aca592e79ef0073fcf3019e5fR383

[michalmuskala]

Some more examples from other issues I've closed in favour of the discussion happening here:

---

From #160

f(1,
    2). % comment

formatted today as:

f(
    1,
    % comment
    2
).

Where the issue author would prefer

% comment
f(
    1,
    2
).

or

f(
    1,
    2
). % comment

---

From #165

[x % x
, y % y
].

formatted today as:

% x
[
    x,
    % y
    y
].

Where the issue author would prefer

[
    % x
    x,
    % y
    y
].

[awalterschulze]

Another data point https://github.com/erlang/otp/pull/2756/files/f3c7a3350952b7d0786d9c5baa5d2c9b8805aa9b#diff-fbfd7bb2c1f79df2b42a31516bd6d8a9R168

[awalterschulze]

After doing more analyses on comments, we have decided that adding support for trailing comments is a good idea #219

Doing it will require a lot of work, so no promises on when this will be delivered, but I believe we can consider this issue a duplicate, because if we support trailing comments, then this wouldn't have been an issue.

I hope that makes sense.