NixOS/nixfmt

Consider not inserting newlines after assignment equals

Closed this issue · 1 comments

RuRo commented

Description

In my opinion, inserting a line break after an = in an assignment is almost always pointless and makes the code less readable. If the RHS of the assignment can itself be split into multiple lines, we should prefer to do that (see issue #136). If the RHS can't be split (for example, if it's a long literal), then I would argue that it's better to just leave it on a single line.

Small example input

not-the-bees: {

  url = "https://github.com/this_man_does_not_exist/best_repo_ever/compare/0123456789abcdef0123456789abcdef01234567..0000111122223333444455556666777788889999.patch";

  path = ./node_modules/this/path/is/so/incredibly/long/that/we/have/absolutely/no/hope/of/ever/fitting/it/on/a/single/line/like/its/completely/pointless/to/even/try.js;

  this = not-the-bees.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;
}

Expected output

not-the-bees: {

  url = "https://github.com/this_man_does_not_exist/best_repo_ever/compare/0123456789abcdef0123456789abcdef01234567..0000111122223333444455556666777788889999.patch";

  path = ./node_modules/this/path/is/so/incredibly/long/that/we/have/absolutely/no/hope/of/ever/fitting/it/on/a/single/line/like/its/completely/pointless/to/even/try.js;

  this = not-the-bees.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;
}

Actual output

not-the-bees: {

  url =
    "https://github.com/this_man_does_not_exist/best_repo_ever/compare/0123456789abcdef0123456789abcdef01234567..0000111122223333444455556666777788889999.patch";

  path =
    ./node_modules/this/path/is/so/incredibly/long/that/we/have/absolutely/no/hope/of/ever/fitting/it/on/a/single/line/like/its/completely/pointless/to/even/try.js;

  this =
    not-the-bees.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;
}

Edge cases and alternative solutions

In these examples, I intentionally didn't include assignments where the RHS is long enough to overflow the character limit on the same line as the assignment, but at the same time short enough to fit on 2 lines (lhs = on one line and rhs; on the next). I would be fine with either formatting for that edge case (although I am personally leaning slightly towards a simple rule of "never insert a newline directly after the = in assignments").

One possible alternative formatting would be to insert parentheses around the chopped-down expression like this:

{
  this = (
    not-the-bees.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  );
}

but I personally don't like this style. It requires modifying non-whitespace characters and overall seems like a "worst of both worlds" kind of solution to me.


P.S. Assignments inside let ... in ... blocks currently work the same as assignments in attrsets. Whatever resolution we choose for this issue, it should equally apply to assignments in let ... in ... blocks.

This is implemented in #118, specifically a7dc8bb