kamadorueda / alejandra

The Uncompromising Nix Code Formatter
https://kamadorueda.github.io/alejandra/
The Unlicense
859 stars 41 forks source link

visual change on multi-line string #62

Open zimbatm opened 2 years ago

zimbatm commented 2 years ago

Here is a minimal repo of the issue:

{
  fetch_builtin-url =
    ''[name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
        $ niv modify name -a type=file -a builtin=true'';
}

After formatting:

--- a/repro.nix
+++ b/repro.nix
@@ -1,5 +1,5 @@
 {
   fetch_builtin-url =
-    ''[name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
-        $ niv modify name -a type=file -a builtin=true'';
+    ''      [name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
+              $ niv modify name -a type=file -a builtin=true'';
 }

As you can see, the multi-line string has gained a few whitespaces in front, changing the content of the Nix value.

This is probably related to https://github.com/nix-community/rnix-parser/issues/71

kamadorueda commented 2 years ago

At least from Nix eyes, code is just the same™ at the moment:

nix-repl> ''[name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
                  $ niv modify name -a type=file -a builtin=true''
"[name] The niv type \"builtin-url\" will soon be deprecated. You should instead use `builtin = true`.\n        $ niv modify name -a type=file -a builtin=true"

nix-repl> ''      [name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
                        $ niv modify name -a type=file -a builtin=true''
"[name] The niv type \"builtin-url\" will soon be deprecated. You should instead use `builtin = true`.\n        $ niv modify name -a type=file -a builtin=true"

Certainly this would be very nice to improve!

kamadorueda commented 2 years ago

In case that someone wants to help with this, the solution is introducing some post-processing here: https://github.com/kamadorueda/alejandra/blob/main/src/rules/string.rs#L113

kamadorueda commented 2 years ago

I was looking at this, the visual change happens in the last step,

before the last step, strings are in a fully dedented state, like this:

  ''
mkdir -p "$out/lib/modules/${kernel.modDirVersion}/kernel/net/wireless/"
''

  ''<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
${expr "" v}
</plist>''

  ''[name] The niv type "builtin-url" will soon be deprecated. You should instead use `builtin = true`.
        $ niv modify name -a type=file -a builtin=true''

In the last step, the engine pads them with enough whitespace so that the content is after the opening ''

the trick would be to know in advance how much padding would be needed, and if all lines start with more whitespace than the needed padding, do nothing, they are "already padded"

we would need to move this last step to the builder, which is the only one who knows the current indentation level, and thus the needed padding

zimbatm commented 2 years ago

If the parser was providing the indent as a separate token, it would make that kind of scenario easier. A multi-line string should be a list of (indent, line) pairs