Weird start of let block

[mweinelt]

Before:

assert ltoSupport -> stdenv.isDarwin -> throw "LTO is broken on Darwin (see PR#19312).";

let
  flag = tf: x: [(if tf then "--enable-${x}" else "--disable-${x}")];

After:

assert ltoSupport -> stdenv.isDarwin -> throw "LTO is broken on Darwin (see PR#19312)."; let
  flag = tf: x: [

https://github.com/nixos/nixpkgs/blob/master/pkgs/applications/networking/browsers/firefox/common.nix#L83-L86

I don't think putting that let into the same line as the assert statement is beneficial.

[DavHau]

In general I'm also not a big fan of the let's being pulled up into the preceeding line.

I think the following layout is always preferable:

{
  something =
    let
      foo = "abc";
    in
      function foo;
}

The downsides of it:

• it adds an extra layer of indentation
• more white space in general

But I think the alternative is worse:

{
  something = let
    foo = "abc";
  in
    function foo;
}

Why is this bad? Not everything that belongs to something is indented relative to it. The in keyword is aligned with something, despite being a child of it. On this small example it might not affect readability a lot, but on larger code blocks, with many functions/variables and a lot of let..in's, I find it significantly harder to reason which let..in belongs to which variable. ... but maybe that's just me not being used to that style.

[mweinelt]

I agree that we open up another scope with the let binding, so another indentation-level is fair game.

[DavHau]

Another argument in favor of the squashed/not indented layout is, that whenever you refactor code and add a let..in statement where there wasn't before, then all the existing code gets indented, resulting in larger diffs.

But, I don't think that optimizing for reduced diffs justifies the trade-off in readability in this case

[piegamesde]

(Disclaimer: I am the one who initially requested more in-line let bindings)

Especially at the top level, having the let binding on the new line results in the whole content being indented more than necessary. Even when ignoring the diff aspect for now, there were a few quite ugly results. (I do think general diffability is an important aspect, although I am against minimizing the diff of the initial nixpkgs format)

I agree though that the specific case with the assert is ugly too. Note that if the assert (same applies to with statements btw) is within a function definition rather than an attribute, we could probably spread it on multiple lines without adding an indentation level to the content:

{foo}:
assert foo != null;
let
  bar = foo;
in
  bar

[yajo]

Happened to me while developing a flake.

    inputs.flake-utils.lib.eachDefaultSystem (system: let
      pkgs = import inputs.nixpkgs {
        inherit system;
        overlays = [inputs.devshell.overlay];
      };
    in {
...

Looks weird. One would expect to find a let sentence indented at the same level as the trailing in just looking above of it.

[DavHau]

It would be really nice if this was solved. I think this is a major issue impacting readability.

I think, a multi-line let..in should always start in a new line and be indented relative to the preceding part of the expression. With the current style, this is not the case, therefore making it harder to reason about complex code spanning many lines or even pages, with nested let..in's etc.

I understand that it seems less necessary to indent top-level definitions, so we could make an exception for this case.

What do you think @kamadorueda ?

[winterqt]

@kamadorueda I'm also interested in seeing this addressed -- what do you think? (It's been a bit over 2 months since your opinion was asked for, so I figured I'd ask again just in case you forgot or something. Thanks!)

[kamadorueda]

Personally, I'm more inclined toward what @DavHau says, adding indentation costs 2 spaces of horizontal space, but it certainly improves readability a lot. If I recall correctly this was the original style I created Alejandra with, so we may as well go back to it.

This is the kind of things I'm trying to address by creating the style guide: https://github.com/kamadorueda/alejandra/blob/main/STYLE.md If it's there, it's well thought why and by extension probably the right choice. I'm missing to add the let-in, thanks for helping me gather arguments with this discussion

[gekoke]

{
  # Code elided
  outputs = inputs @ {
    self,
    nixpkgs,
    home-manager,
    ...
  }: let
    # Code elided
  in {
    # Code elided
  };
}

For me, this was also the only thing that stuck out to me when I ran the formatter. I think that let not being on the same level of indentation as in should be exceptional and not the rule.