Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add antiquotations to paths #5066

Merged
merged 6 commits into from
Sep 1, 2021
Merged

add antiquotations to paths #5066

merged 6 commits into from
Sep 1, 2021

Conversation

Radvendii
Copy link
Contributor

@Radvendii Radvendii commented Jul 29, 2021

This enables you to write something like

let
   foo = "foo";
in
  ./${foo}/bar.nix

Without this one has to write

./. + "/${foo}/bar.nix"

Which is less ergonomic. The other intuitive approaches

"${./.}/${foo}/bar.nix"

and

./. + "/" + foo + "bar.nix"

Are semantically different, and usually not what one wants. Adding this syntax provides an obvious, and usually correct way of accessing paths that depend on a variable.

This also affects HPATHs e.g. ~/${foo}/bar

It does not affect SPATHs e.g. <nixpkgs/${foo}> is not valid, even with this PR.

I couldn't figure out any way to do SPATHs without changing the lex of things like 3<a.${foo}, which is currently a valid lex. In theory anything that ends in > would be an error anyways, but in order to find that out one needs to parse the whole expression, including arbitrary nix code in the antiquotation. Given that flakes don't support SPATHs anyways, this seems like a minor sacrifice.

This change shouldn't affect anything that currently passes the lexer. All the tests pass without modification, and the generated nixpkgs derivations are identical.

@Radvendii Radvendii marked this pull request as draft July 29, 2021 16:32
@Radvendii Radvendii marked this pull request as ready for review August 6, 2021 11:54
src/libexpr/eval.cc Outdated Show resolved Hide resolved
@edolstra
Copy link
Member

LGTM, can you add some documentation about this to doc/manual/src/expressions/language-values.md?

I don't think this changes the way any program would behave, but it's a
cleaner internal representation.
@Radvendii
Copy link
Contributor Author

The failure on ubuntu seems to be due to some unrelated issue with recursive nix and network connection? I could be misinterpreting that though.

@edolstra edolstra merged commit 7ee639f into NixOS:master Sep 1, 2021
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/tweag-nix-dev-update-17/15037/1

@cstrahan
Copy link
Contributor

cstrahan commented Mar 18, 2022

@Radvendii

Note that this assertion in the docs is wrong:

a.${foo}/b.${bar} is a syntactically valid division operation.

nix-repl> x = { foo = arg: arg; }

nix-repl> foo = "foo"

nix-repl> bar = "bar"

nix-repl> x.${foo}/x.${bar}
/x.bar

The x.${foo} gets parsed as an attribute selection, to which /x.${bar} (which evaluates as an /x.bar path) is applied.

This is actually how my https://github.com/cstrahan/tree-sitter-nix grammar parses this as well, so I don't really mind -- I think it's a reasonable parse, after all. I think it makes sense that the second /x. would be lexed as the start of a path -- trying to tweak the lexer and/or parser to treat this as division would be kinda gross, IMO.

I discovered this while adding another test case (inspired by the docs):

====================
division not mistaken for paths (2)
====================

a.${foo}/b.${bar}

---

(source_expression
    (binary
        (select
            (identifier)
            (attrpath
            (interpolation
                (identifier))))
        (select
            (identifier)
            (attrpath
            (interpolation
                (identifier))))))

turns out, the actual parse looks like:

(source_expression
    (app
        (select
            (identifier)
            (attrpath
            (interpolation
                (identifier))))
        (path
            (path_fragment)
            (interpolation
                (identifier)))))

@Radvendii
Copy link
Contributor Author

@cstrahan Oh, you're totally right. It doesn't get parsed as a single path, but it also does not get parsed as a division.

I'm not sure exactly how to clarify that in the docs without introducing more confusion.

Unfortunately, I think what makes the most sense is indeed to parse a.${foo}/b.${bar} as a single path, but this wouldn't be backwards compatible. I guess we could say that:

-    Antiquotation is supported in any paths except those in angle brackets.
-    `./${foo}-${bar}.nix` is a more convenient way of writing 
-    `./. + "/" + foo + "-" + bar + ".nix"` or `./. + "/${foo}-${bar}.nix"`. At
-    least one slash must appear *before* any antiquotations for this to be
-    recognized as a path. `a.${foo}/b.${bar}` is a syntactically valid division
-    operation. `./a.${foo}/b.${bar}` is a path.
+    Antiquotation is supported in any paths except those in angle brackets.
+    `./${foo}-${bar}.nix` is a more convenient way of writing 
+    `./. + "/" + foo + "-" + bar + ".nix"` or `./. + "/${foo}-${bar}.nix"`. At
+    least one slash must appear *before* any antiquotations for this to be
+    recognized as a path. For backward compatibility, `a.${foo}/bar` is 
+    parsed as `a.${foo}` applied to the path `/bar`. To make a single path,
+    you can always prepend a slash: `/a.${foo}/bar`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants