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

Remove unthrown error from function argument set example #46

Merged
merged 1 commit into from Apr 11, 2018

Conversation

jackjennings
Copy link
Contributor

Not sure if this is an issue in the documentation or an irregularity in nix/nix-repl I'm running, but I'm not seeing the error that this example describes:

~ > nix-repl
Welcome to Nix version 1.11.15. Type :? for help.

nix-repl> mul = s: s.a*s.b

nix-repl> mul { a = 3; b = 4; c = 6; }
12

According to the example, this should throw an error because c is an unused key.

Running under Nix 1.11.15 on macOS.

@grahamc
Copy link
Member

grahamc commented Jan 28, 2018

Hmm the code block prior to it shows a separate definition of mul where you should get that error:

nix-repl> mul = { a, b }: a*b
nix-repl> mul { a = 3; b = 4; }
12

Do you have ideas on how to clear that up?

@dtzWill
Copy link
Member

dtzWill commented Mar 11, 2018

Use a different name for the second variant? mul -> mul2 or so? 😇

It's not immediately obvious which previous `mul` definition applies,
and is a copy-paste hazard. Staring the block with a `mul = ...` is also
consistent with the other code blocks.

Fixes NixOS#46
@Ericson2314
Copy link
Member

Ok I just redefined mul. That's how things were done elsewhere in this pill so this seems to be the most consistent to me. We could give all the muls separate names as a separate change if we like.

@Ericson2314 Ericson2314 merged commit e5534d4 into NixOS:master Apr 11, 2018
@jackjennings jackjennings deleted the patch-3 branch April 12, 2018 00:08
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.

None yet

4 participants