Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Wrong indentation on multi line if/then/else expression #262

Open
DavHau opened this issue Sep 30, 2021 · 4 comments
Open

Wrong indentation on multi line if/then/else expression #262

DavHau opened this issue Sep 30, 2021 · 4 comments
Labels
bug Something isn't working formatting Issue with the output format

Comments

@DavHau
Copy link
Member

DavHau commented Sep 30, 2021

Describe the bug
the following source:

let
  nix = if !buildNix then null else
    buildRacketNix;
in
{ }

... is formatted to

let
  nix = if !buildNix then null else
  buildRacketNix;
in
{ }

Expected behavior
A continuation of a expression in the next line should always be indented.
System information

  • nixpkgs-fmt 1.2.0
@DavHau DavHau added bug Something isn't working needs triage labels Sep 30, 2021
@r-burns
Copy link

r-burns commented Oct 29, 2021

Alternatively, maybe nixpkgs-fmt should move the trailing else to the next line before autoindenting. It appears that newline-before-else is much more common:

$ rg 'else$' | wc -l
1894

$ rg '^ *else' | wc -l
3870

Most of the cases of newline-after-else are actually because the else keyword is alone on the line.

$ rg '^ *else$' | wc -l
1193

@zimbatm
Copy link
Member

zimbatm commented Oct 29, 2021

Or maybe it should add \n after the =:

let
  nix =
    if !buildNix then null else
    buildRacketNix;
in
{ }

@zimbatm
Copy link
Member

zimbatm commented Oct 29, 2021

I agree that the current re-formatting is not ideal.

@zimbatm zimbatm added formatting Issue with the output format and removed needs triage labels Oct 29, 2021
@DavHau
Copy link
Member Author

DavHau commented Oct 30, 2021

Or maybe it should add \n after the =:

let
  nix =
    if !buildNix then null else
    buildRacketNix;
in
{ }

In my opinion yes. Whenever a = is followed by an expression spanning multiple lines, there should always be a \n after the =. Because otherwise those lines cannot be logically aligned correctly to each other. Especially with larger expressions this is very important for readability.

Also I totally think that if/else should almost always be formatted like this:

if bool_expression1 then
  indented_child_expression1
else if bool_expression2 then
  indented_child_expression2
else
  indented_child_expression3

Then again, there is the question of how much freedom the developer should be given. There are always some special cases where an alternative format can make sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working formatting Issue with the output format
Projects
None yet
Development

No branches or pull requests

3 participants