NixOS / nixfmt

The official (but not yet stable) formatter for Nix code
https://nixfmt.serokell.io
Mozilla Public License 2.0
934 stars 41 forks source link

Escape sequences not handled properly #189

Open thefossguy opened 7 months ago

thefossguy commented 7 months ago

Description

It appears that nixfmt does not preserve backslashes. I have the following two lines in my darwin.nix:

https://github.com/thefossguy/prathams-nixos/blob/6acda3f9c27516b81a29d718ea1cb3b1025f86b6/nixos-configuration/home-manager/darwin.nix#L56-L57

And this got mangled like so:

-      "^\Uf702" = "moveWordLeft:"; # Ctrl-<Left>
-      "^\Uf703" = "moveWordRight:"; # Ctrl-<Right>
+      "^Uf702" = "moveWordLeft:"; # Ctrl-<Left>
+      "^Uf703" = "moveWordRight:"; # Ctrl-<Right>
piegamesde commented 7 months ago

These strings are semantically equivalent in Nix:

nix-repl> "^\Uf702"
"^Uf702"
nix-repl> "^\Uf702" == "^Uf702"
true

Not sure if nixfmt should preserve the input regardless or not. On the one hand I want it to treat strings as opaque and never change them, on the other hand normalizing this here has the potential of uncovering very subtle bugs.

thefossguy commented 7 months ago

I don't even remember why I have that key specific string in the key binding but it looks like Nix is actually writing the "incorrect" string to the target file.

https://github.com/nix-community/home-manager/blob/e84811035d7c8ec79ed6c687a97e19e2a22123c1/modules/targets/darwin/keybindings.nix#L41-L42

Contents of ~/Library/KeyBindings/DefaultKeyBinding.dict:

<?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">
<dict>
    <key>^Uf702</key>
    <string>moveWordLeft:</string>
    <key>^Uf703</key>
    <string>moveWordRight:</string>
</dict>
</plist>

And it works. So not an issue at all. Sorry for the trouble :)

infinisil commented 7 months ago

Not sure if nixfmt should preserve the input regardless or not. On the one hand I want it to treat strings as opaque and never change them, on the other hand normalizing this here has the potential of uncovering very subtle bugs.

I think we should preserve the input regardless, because it's not nixfmt's job to fix these kind of problems. Instead it should be Nix itself that does that.

And we do have that in the standard:

The non-interpolated string parts must be preserved from the input.

And by preserving this we can also simplify the code because we don't need to think as much about how Nix parses strings.

If you don't mind, let's leave this open for now :)

infinisil commented 7 months ago

We very briefly discussed this in todays team meeting:

nixos-discourse commented 7 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-04-16/43533/1

nixos-discourse commented 5 days ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/formatting-team-meeting-2024-11-26/56643/1