String interpolation only absorbs when nothing is in front of the antiquotation marker

[Atemu]

Description

When there is nothing before the antiquotation marker ${ (i.e. only string context indent), multi-line constructs are absorbed right behind the antiquotation marker as expected. When there is any string before the marker though, nothing is absorbed.

Repro'd on 923d379.

Small example input

''
  ${somefunc { multiple = true; attrs = false; }}

  short ${somefunc { multiple = true; attrs = false; }}

  loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong ${somefunc { multiple = true; attrs = false; }}
''

Expected output

''
  ${somefunc {
    multiple = true;
    attrs = false;
  }}

  short ${somefunc {
    multiple = true;
    attrs = false;
  }}

  loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong ${somefunc {
    multiple = true;
    attrs = false;
  }}
''

Or perhaps do wrap with the long prefix.

Perhaps splitting across lines should be a little less eager in string interpolations to begin with though so that the first two are just a single line unless that line becomes too long. There usually isn't the same expectation for many more elements to be added inside of an interpolation and it's usually worth the potential diff noise IMHO.

This would be ideal IMHO:

''
  ${somefunc { multiple = true; attrs = false; }}

  short ${somefunc { multiple = true; attrs = false; }}

  loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong ${
    somefunc {
      multiple = true;
      attrs = false;
    }
  }
''

Actual output

''
  ${somefunc {
    multiple = true;
    attrs = false;
  }}

  short ${
    somefunc {
      multiple = true;
      attrs = false;
    }
  }

  loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong ${
    somefunc {
      multiple = true;
      attrs = false;
    }
  }
''

[piegamesde]

That distinction was made intentionally, because in more realistic code examples I found the absorbing to be rather confusing.

[Atemu]

Could you share some examples? This behaviour was confusing to me in every instance I encountered it so far except for long prefixes where I think it's even preferable. I also haven't seen many instances though.

I encountered this in

https://github.com/nix-community/robotnix/blob/6748859cb4c7ffcceac4a302a458d3816ad4ba35/modules/release.nix#L206-L210

If you format this file, three similar interpolations are absorbed but not this one because it's one indent further. It looks really weird and out of place:

            commands =
              ''
                echo Signing target files
                ${signedTargetFilesScript {
                  targetFiles = unsignedTargetFiles;
                  out = signedTargetFiles.name;
                }}
                echo Building OTA zip
                ${otaScript {
                  targetFiles = signedTargetFiles.name;
                  out = ota.name;
                }}
                if [[ ! -z "$PREV_BUILDNUMBER" ]]; then
                  echo Building incremental OTA zip
                  ${
                    otaScript {
                      targetFiles = signedTargetFiles.name;
                      prevTargetFiles = "${config.device}-target_files-$PREV_BUILDNUMBER.zip";
                      out = "${config.device}-incremental-$PREV_BUILDNUMBER-${config.buildNumber}.zip";
                    }
                  }
                fi
                echo Building .img file
                ${imgScript {
                  targetFiles = signedTargetFiles.name;
                  out = img.name;
                }}
                echo Building factory image
                ${factoryImgScript {
                  targetFiles = signedTargetFiles.name;
                  img = img.name;
                  out = factoryImg.name;
                }}
              ''
              + lib.optionalString config.apps.updater.enable ''
                echo Writing updater metadata
                ${writeOtaMetadata {
                  otaFile = ota.name;
                  path = ".";
                }}
              '';

[piegamesde]

I see. While the line break on non-empty lines is intentional, whitespace should not count as "content" towards that metric.

[Atemu]

I built a regex that matches instances of this case:

^\S+.*\$\{[^}]*$

Some instances:

Original:

"src/third_party/llvm-build/Release+Asserts" = runCommand "download_upstream_clang" {} ''
    mkdir $out
    tar xf ${fetchurl {
                url  = "https://commondatastorage.googleapis.com/chromium-browser-clang/Linux_x64/clang-llvmorg-15-init-1995-g5bec1ea7-1.tgz";
                sha256 = "0pxx8jr958xi5szxl5hc7yq6gmppg1paw25v4myfnqb62gjzik62";
            }} -C $out
'';

nixfmt:

  "src/third_party/llvm-build/Release+Asserts" = runCommand "download_upstream_clang" { } ''
    mkdir $out
    tar xf ${
      fetchurl {
        url = "https://commondatastorage.googleapis.com/chromium-browser-clang/Linux_x64/clang-llvmorg-15-init-1995-g5bec1ea7-1.tgz";
        sha256 = "0pxx8jr958xi5szxl5hc7yq6gmppg1paw25v4myfnqb62gjzik62";
      }
    } -C $out
  '';

What I proposed:

  "src/third_party/llvm-build/Release+Asserts" = runCommand "download_upstream_clang" { } ''
    mkdir $out
    tar xf ${fetchurl {
      url = "https://commondatastorage.googleapis.com/chromium-browser-clang/Linux_x64/clang-llvmorg-15-init-1995-g5bec1ea7-1.tgz";
      sha256 = "0pxx8jr958xi5szxl5hc7yq6gmppg1paw25v4myfnqb62gjzik62";
    }} -C $out
  '';

But I could see an argument against this in this particular case because there's something behind the interpolation too.

<nixpkgs>/ci/request-reviews/default.nix:

        --set PATH ${
          lib.makeBinPath [
            coreutils
            codeowners
            jq
            curl
            github-cli
            gitMinimal
          ]
        }

vs.

        --set PATH ${lib.makeBinPath [
          coreutils
          codeowners
          jq
          curl
          github-cli
          gitMinimal
        ]}

This looks really bad to my eye. I think that's mostly due to the characters above the indent being punctuation though. Something like

        doSet PATH ${lib.makeBinPath [
          coreutils
          codeowners
          jq
          curl
          github-cli
          gitMinimal
        ]}

would look absolutely fine.

<nixpkgs>/lib/strings.nix:

      "declare -A ${name}=(${
        concatStringsSep " " (lib.mapAttrsToList (n: v:
          "[${escapeShellArg n}]=${escapeShellArg v}"
        ) value)
      })"

vs.

      "declare -A ${name}=(${concatStringsSep " " (lib.mapAttrsToList (n: v:
        "[${escapeShellArg n}]=${escapeShellArg v}"
      ) value)})"

Not bad but it gets really quite crowded with closing brackets in the bottom. If we have to go multi-line anyways, we might as well use a little more space unless it looks very odd.

<nixpkgs>/nixos/modules/services/web-apps/agorakit.nix:

          mkSecretReplacement = file: ''
            replace-secret ${
              escapeShellArgs [
                (builtins.hashString "sha256" file)
                file
                "${cfg.dataDir}/.env"
              ]
            }
          '';

vs.

          mkSecretReplacement = file: ''
            replace-secret ${escapeShellArgs [
              (builtins.hashString "sha256" file)
              file
              "${cfg.dataDir}/.env"
            ]}
          '';

This list arg absorbtion looks a lot better than the first example. I think I'd prefer absorbed here. Non-absorbed isn't that bad either though.

<nixpkgs>/pkgs/by-name/gk/gk-cli/package.nix:

      export PATH="${
        lib.makeBinPath [
          curl
          jq
          common-updater-scripts
        ]
      }"

vs.

      export PATH="${lib.makeBinPath [
        curl
        jq
        common-updater-scripts
      ]}"

This list absorption looks quite nice too though. I don't hate the non-absorbed style though.

<nixpkgs>/pkgs/os-specific/linux/minimal-bootstrap/stage0-posix/make-bootstrap-sources.nix:

    patch -Np1 -d $out/M2libc -i ${
      (fetchpatch {
        url = "https://github.com/oriansj/M2libc/commit/ff7c3023b3ab6cfcffc5364620b25f8d0279e96b.patch";
        hash = "sha256-QAKddv4TixIQHpFa9SVu9fAkeKbzhQaxjaWzW2yJy7A=";
      })
    }

vs.

    patch -Np1 -d $out/M2libc -i ${(fetchpatch {
      url = "https://github.com/oriansj/M2libc/commit/ff7c3023b3ab6cfcffc5364620b25f8d0279e96b.patch";
      hash = "sha256-QAKddv4TixIQHpFa9SVu9fAkeKbzhQaxjaWzW2yJy7A=";
    })}

Again spicy closing brackets but a little more snug.

The parens are superfluous in this case though:

    patch -Np1 -d $out/M2libc -i ${fetchpatch {
      url = "https://github.com/oriansj/M2libc/commit/ff7c3023b3ab6cfcffc5364620b25f8d0279e96b.patch";
      hash = "sha256-QAKddv4TixIQHpFa9SVu9fAkeKbzhQaxjaWzW2yJy7A=";
    }}

This looks quite nice again.

<nixpkgs>/pkgs/by-name/br/brise/fetchPackages.nix:

  ln -sv ${
    fetchFromGitHub {
      owner = "rime";
      repo = "rime-array";
      rev = "d10f2f8b2aec7c7e736ace01e8a399e5ae5e7c3a";
      sha256 = "sha256-4t6+gh2V57SueDp9Tn6vTuxQCZNGzjLdJEhzIEqRjdI=";
    }
  } array

vs.

  ln -sv ${fetchFromGitHub {
    owner = "rime";
    repo = "rime-array";
    rev = "d10f2f8b2aec7c7e736ace01e8a399e5ae5e7c3a";
    sha256 = "sha256-4t6+gh2V57SueDp9Tn6vTuxQCZNGzjLdJEhzIEqRjdI=";
  }} array

Again slight preference to the latter though I'm wary that it might be easier to miss the array suffix.

---

Having seen more examples, I'm a bit more convinced of the current non-absorbed style. There are other arguments to consider though such as uniformity with other places but that also goes the other way as many interpolations I came across cannot be absorbed at all such as ternaries.

I think if it's a whitespace-only prefix, it's pretty clearly preferable to absorb though:

^\s+\$\{[^}]*$

<nixpkgs>/pkgs/servers/web-apps/bookstack/composer-env.nix:

              {
                  $config = json_decode($composerLockStr, true);

                  if(array_key_exists("packages", $config))
                      $allPackages = $config["packages"];
                  else
                      $allPackages = array();

                  ${
                    lib.optionalString (!noDev) ''
                      if(array_key_exists("packages-dev", $config))
                          $allPackages = array_merge($allPackages, $config["packages-dev"]);
                    ''
                  }

vs.

              {
                  $config = json_decode($composerLockStr, true);

                  if(array_key_exists("packages", $config))
                      $allPackages = $config["packages"];
                  else
                      $allPackages = array();

                  ${lib.optionalString (!noDev) ''
                    if(array_key_exists("packages-dev", $config))
                        $allPackages = array_merge($allPackages, $config["packages-dev"]);
                  ''}

<nixpkgs>/pkgs/by-name/br/brave/make-brave.nix:

    gappsWrapperArgs+=(
      ...
      ${
        optionalString (enableFeatures != [ ]) ''
          --add-flags "--enable-features=${strings.concatStringsSep "," enableFeatures}\''${NIXOS_OZONE_WL:+\''${WAYLAND_DISPLAY:+,WaylandWindowDecorations}}"
        ''
      }

vs.

    gappsWrapperArgs+=(
      ...
      ${optionalString (enableFeatures != [ ]) ''
        --add-flags "--enable-features=${strings.concatStringsSep "," enableFeatures}\''${NIXOS_OZONE_WL:+\''${WAYLAND_DISPLAY:+,WaylandWindowDecorations}}"
      ''}

I also found this:

<nixpkgs>/pkgs/by-name/ya/yazi/package.nix:

        mkdir $out/plugins
        ${lib.optionalString (plugins != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/plugins/${name}") plugins
          )}
        ''}

        mkdir $out/flavors
        ${lib.optionalString (flavors != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/flavors/${name}") flavors
          )}
        ''}

which counter-intuitively does absorb but that's because it's a nested multi-line string and the interpolation is on the first indent of that. Absorbing in whitespace-only prefixes would be more uniform with this case too; e.g.:

        mkdir $out/plugins
        ${lib.optionalString (plugins != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/plugins/${name}") plugins
          )}
        ''}
        if [ -z $SOMEVAR ];
          ${
            lib.funcFoo {
              many = [ ];
              arguments = { };
            }
          }
        fi
        mkdir $out/flavors
        ${lib.optionalString (flavors != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/flavors/${name}") flavors
          )}
        ''}

vs.

        mkdir $out/plugins
        ${lib.optionalString (plugins != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/plugins/${name}") plugins
          )}
        ''}
        if [ -z $SOMEVAR ];
          ${lib.funcFoo {
            many = [ ];
            arguments = { };
          }}
        fi
        mkdir $out/flavors
        ${lib.optionalString (flavors != { }) ''
          ${lib.concatStringsSep "\n" (
            lib.mapAttrsToList (name: value: "ln -s ${value} $out/flavors/${name}") flavors
          )}
        ''}

[Atemu]

Thank you!