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

libexpr: experimental pipe operators #11131

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

rhendric
Copy link
Member

@rhendric rhendric commented Jul 17, 2024

Motivation

A draft implementation of NixOS/rfcs#148 (excluding builtins.pipe).

Made the following choices about precedence and associativity:

  • |> is left-associative and <| is right-associative
  • These two operators do not associate with each other; (x: 1) <| 2 |> (x: 3) is a parse error
  • Both operators are weaker than ->, the previous weakest operator (part of the motivation here is to reduce parentheses, so the weaker these operators are, the more situations in which they can be used instead of parentheses)

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@rhendric rhendric requested a review from edolstra as a code owner July 17, 2024 16:50
src/libutil/experimental-features.cc Outdated Show resolved Hide resolved
tests/unit/libexpr/main.cc Show resolved Hide resolved
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add it to the list of operators, to show the precedence? It's in doc/manual/src/language/operators.md

src/libexpr/lexer.l Outdated Show resolved Hide resolved
@rhendric rhendric force-pushed the rhendric/pipe-operators branch from 1012bb9 to bbb6e25 Compare July 17, 2024 21:05
@github-actions github-actions bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Jul 17, 2024
@rhendric rhendric force-pushed the rhendric/pipe-operators branch 2 times, most recently from a640be3 to 0f0803b Compare July 17, 2024 21:32
src/libutil/experimental-features.cc Outdated Show resolved Hide resolved
doc/manual/rl-next/pipe-operators.md Outdated Show resolved Hide resolved
This is a draft implementation of [RFC 0148](https://github.com/NixOS/rfcs/pull/148).

The `pipe-operators` experimental feature adds `<|` and `|>` operators to the Nix language.
*a* `|>` *b* is equivalent to the function application *b* *a*, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: "a into b", should we give it a name like that? Could be nice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷 The other direction is a little harder to name, I think? I don't know if this is important.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"into" follows nicely from the more explicit "a piped into b", though I'm not sure about the other direction (<|).

Maybe that could be read as "from", as in a <| b is "a piped from b"; although I'd personally just read it backwards as "b piped into a".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"taking"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or ‘of’?

This seems like tutorial stuff, not reference stuff; these names would be entirely non-normative and not necessary for understanding the language from first principles. So does it belong in the reference documentation, or in release notes, or both or neither?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or ‘of’?

This seems like tutorial stuff, not reference stuff; [...] entirely non-normative

Certainly. It was just a

thought

I figured it'd be good to talk about this.


I think I like these two, because they're short and literal, with a word per symbol, in order.

  • |> pipe into
  • <| back pipe

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late to the party, but FWIW in the current Lix draft I call them pipe left / pipe right in the code and tend towards calling them pipe (forward) and pipe backward in the documentation. (The forward being implicit and sometimes omitted)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to add some more ideas into the pot, a plumber would usually talk about pipes "feeding" or "supplying" and then "draining" into the wastewater pipes.

E.g:

  • "this tap is fed from the cold water supply"
  • "the sink drains down a 50mm pipe"
  • "the bath is supplied hot water from pipe A"
  • "pipe B supplies the bath with cold water"
  • "pipe C feeds the shower with cold water"
  • "the shower is fed hot water by pipe D"

I don't think this will sound natural in the context of programming, but I thought it was worth reminiscing on my background in construction 😅

I agree "Pipe left/backward" and "pipe right/forward/into" should be fairly natural, simple and obvious

doc/manual/rl-next/pipe-operators.md Show resolved Hide resolved
@rhendric rhendric force-pushed the rhendric/pipe-operators branch from 0f0803b to bf0570e Compare July 17, 2024 21:36
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

tests/unit/libexpr/main.cc Show resolved Hide resolved
This is a draft implementation of [RFC 0148](https://github.com/NixOS/rfcs/pull/148).

The `pipe-operators` experimental feature adds `<|` and `|>` operators to the Nix language.
*a* `|>` *b* is equivalent to the function application *b* *a*, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"taking"?

`<|` and `|>` are right and left associative, respectively, and have lower precedence than any other operator.
These properties may change in future releases.

See [the RFC](https://github.com/NixOS/rfcs/pull/148) for more examples and rationale.
Copy link
Member

@roberth roberth Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Channeling @fricklerhandwerk, the reference docs should be at least as complete as this.
EDIT (except for the temporal parts)

@rhendric rhendric force-pushed the rhendric/pipe-operators branch from bf0570e to 43be67e Compare July 17, 2024 21:58
@edolstra
Copy link
Member

We should be pretty reluctant to add syntactic sugar like this, since it makes the Nix language (even) harder to learn. Such features may be attractive to power users, but increase the barrier to entry for new users, who will have to look up another bit of non-standard syntax. (The use of |> rather than | makes it not super-obvious that this is a pipe operator, unless you're an F# user.)

Since this adds it as an experimental feature, what would be the criterion for stabilizing it?

@MattSturgeon
Copy link

We should be pretty reluctant to add syntactic sugar like this, since it makes the Nix language (even) harder to learn. Such features may be attractive to power users, but increase the barrier to entry for new users, who will have to look up another bit of non-standard syntax. (The use of |> rather than | makes it not super-obvious that this is a pipe operator, unless you're an F# user.)

Hasn't this been discussed already on the RFC? (e.g. NixOS/rfcs#148 (comment))

Since this adds it as an experimental feature, what would be the criterion for stabilizing it?

I would assume the criteria for stabilizing would be the RFC getting merged?

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-07-17-nix-team-meeting-minutes-162/49255/1

@rhendric
Copy link
Member Author

We should be pretty reluctant to add syntactic sugar like this, since it makes the Nix language (even) harder to learn.

I would say going through the RFC process and gating the implementation behind an experimental feature flag is an appropriate amount of reluctance. What are experimental features for, if we can't do experiments with them?

We can talk all day year about how hard it is to learn different syntaxes, but the proof is in actually trying to teach them to people. It's much easier to do that with an implementation. Otherwise we are stuck in an endless chicken-and-egg loop where the feature doesn't get implemented because people are skeptical about how it will be received, and nobody can assuage the skeptics by trying it out with people at various experience levels because it isn't implemented.

RFC 0148 is currently the highest-thumbed open RFC, with virtually no negative reactions. There is clearly significant interest in the community in seeing some version of this proposal move forward. The original author of the RFC has already expressed their interest in pursuing an implementation in a competing fork of Nix. Is that your vision of how Nix should position itself—‘like that other implementation, but stuck with the syntax from ten years ago!’ Wouldn't it be better if we could run the test and see if the broader, non-power-user community likes this proposal without telling people they have to use a competing implementation to try it out?

| expr_op
;

expr_apply
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between expr_apply and expr_app? It's a bit confusing to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could do

Suggested change
expr_apply
expr_pipe_apply

and expr_pipe_apply_flipped, expr_pipe_into or expr_pipe_forward for the other production.

Copy link
Member Author

@rhendric rhendric Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed the tokens and nonterminals. Please let's not get hung up on making the names perfect; these are internal names that can be improved at any time, whereas releasing this feature is something that other people are waiting for.

@rhendric rhendric force-pushed the rhendric/pipe-operators branch from 43be67e to e086d5d Compare July 24, 2024 17:34
@roberth roberth merged commit 6ec123a into NixOS:master Jul 25, 2024
12 checks passed
@roberth
Copy link
Member

roberth commented Jul 25, 2024

Thank you @rhendric!

@rhendric rhendric deleted the rhendric/pipe-operators branch July 25, 2024 15:34
@Aleksanaa
Copy link
Member

The speed at which nix team process PR is changing from one extreme to the other 🤯

@roberth
Copy link
Member

roberth commented Jul 25, 2024

To manage expectations, this is a very small and controlled feature, in part thanks to its experimental status and preparation in the form of design discussions on an RFC.
Not to say that those are prerequisites either, by the way. There's other ways to make easy to accept contributions, such as fixes (esp. the kind that doesn't sacrifice something else), or picking up one of the issues in pre-approved directions.
I do agree we're picking up pace though 😃

Mic92 pushed a commit to Mic92/nix-1 that referenced this pull request Jul 25, 2024
Arose because NixOS#9014 merged before
NixOS#11131, but the latter did not rebase /
merge against the latest master.
rhendric pushed a commit to rhendric/nix that referenced this pull request Jul 25, 2024
Arose because NixOS#9014 merged before
NixOS#11131, but the latter did not rebase /
merge against the latest master.
@roberth
Copy link
Member

roberth commented Jul 26, 2024

We should be pretty reluctant to add syntactic sugar like this, since it makes the Nix language (even) harder to learn.

I am absolutely sympathetic to this, but we have to consider the whole system, including lib and its design.
For context, here's my thoughts on a recent PR.
I'd recommend you to read the whole thing, but the crux of it is that the pipe operator(s) remove a strong incentive to add many "convenience" functions that ultimately make the library as a whole harder to comprehend, both when learning the library, and when reading or updating code that uses it.
In this larger context, one or two operators are a small price to pay for a significantly leaner and more effective library.

@anna328p
Copy link
Member

anna328p commented Oct 8, 2024

There's a small surprise that I noticed: I expected the operators to act like composition and reverse composition, so I could write something like pairsToSet = listToAttrs <| map nameValuePair. but this is not how they behave, and I was confused for a while until I realized it corresponded to listToAttrs (map nameValuePair) and I couldn't write point-free code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants