Skip to content

doc: add styleguide on use of with to CONTRIBUTING.md#294383

Draft
lolbinarycat wants to merge 4 commits intoNixOS:masterfrom
lolbinarycat:with-styleguide
Draft

doc: add styleguide on use of with to CONTRIBUTING.md#294383
lolbinarycat wants to merge 4 commits intoNixOS:masterfrom
lolbinarycat:with-styleguide

Conversation

@lolbinarycat
Copy link
Copy Markdown
Contributor

@lolbinarycat lolbinarycat requested a review from infinisil as a code owner March 9, 2024 00:24
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Mar 9, 2024
Copy link
Copy Markdown
Contributor

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

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

I think as-is, this is too narrow of a scope (focusing on meta only). Perhaps this should be phrased to discourage with in any scope where its use may cause confusion. Clarifying the use in meta within a couple of bullets is fine to me, as long as we also explain (and focus on) the general policy.

CONTRIBUTING.md Outdated

#### Use of `with`

`with` is a somewhat controversial feature of nix syntax, it can reduce repition, but easily can make code unreadable or buggy if used carelessly.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

perhaps link to the nix.dev documentation here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i considered it, but honestly, i don't think very highly of that guide, as it frankly lacks a lot of nuance, and feels like it might as well call with a deprecated feature. it doesn't provide any examples of times it would be ok or even encouraged to use.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That guide can be improved too: https://github.com/NixOS/nix.dev

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be great to improve the article on with with acceptable uses and a better explanation of the problems. We can also link it from here for completeness already though.

I think this contributing section here is still valuable since it's more Nixpkgs specific and doesn't necessarily apply to Nix in general.

Co-authored-by: éclairevoyant <848000+eclairevoyant@users.noreply.github.com>
@AndersonTorres
Copy link
Copy Markdown
Member

Voicing my opinion as a with-hater, I am against merging this pull request.
The merits of getting rid of with are mostly technical, not stylistic.

If the technical merits of banishing with to the most limited scopes (namely, a single line) don't convince everyone, then it is not good to force it over everyone - and way worse is to force it as a style.

@lolbinarycat
Copy link
Copy Markdown
Contributor Author

@AndersonTorres this PR does not "banish" the use of with at all. instead it only restricts its usage in specific scenarios that are widely considered confusing and hard to read.

for example, nested with makes it hard to tell where an identifier is coming from.

@lolbinarycat lolbinarycat marked this pull request as draft March 13, 2024 16:12
Copy link
Copy Markdown
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Discussed in the docs team meeting today.

I don't have much of an opinion here, I'm also not a fan of with, and don't think it would be problematic to have it in the style guide, but I can also understand @AndersonTorres' arguments.

CONTRIBUTING.md Outdated

#### Use of `with`

`with` is a somewhat controversial feature of nix syntax, it can reduce repition, but easily can make code unreadable or buggy if used carelessly.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be great to improve the article on with with acceptable uses and a better explanation of the problems. We can also link it from here for completeness already though.

I think this contributing section here is still valuable since it's more Nixpkgs specific and doesn't necessarily apply to Nix in general.

@nixos-discourse
Copy link
Copy Markdown

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

https://discourse.nixos.org/t/2024-03-14-documentation-team-meeting-notes-113/41462/1

lolbinarycat and others added 2 commits March 14, 2024 16:03
Co-authored-by: Silvan Mosberger <github@infinisil.com>
Co-authored-by: Silvan Mosberger <github@infinisil.com>
@Qyriad Qyriad added 6.topic: best practices Documentation and discussion around best practices for Nixpkgs development 6.topic: documentation Meta-discussion about documentation and its workflow labels May 26, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 1, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
Copy link
Copy Markdown
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

I'd say it would be better to try to put these into exact rules and encode them, for example via nixpkgs-vet. Could be a ratchet check first and later a full check.

cc @NixOS/nixpkgs-vet

Comment on lines +752 to +754
acceptable uses:
- `meta = with lib; { ... }`
this allows you to not have to write `lib` twice for `licenses` and `maintainers`, at the cost of nesting `with`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the big effort going on to move away from meta = with lib;, this is certainly not "acceptable" anymore, today.

@nixpkgs-ci nixpkgs-ci bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 7, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: best practices Documentation and discussion around best practices for Nixpkgs development 6.topic: documentation Meta-discussion about documentation and its workflow 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation: style guide for use of with

8 participants