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

lib.path.splitRoot: init #244358

Merged
merged 2 commits into from
Jul 27, 2023
Merged

lib.path.splitRoot: init #244358

merged 2 commits into from
Jul 27, 2023

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Jul 19, 2023

Description of changes

Creates a new function:

  • lib.path.splitRoot :: Path -> { root :: Path, subpath :: String }: Split a path into its parts.
    lib.path.splitRoot /foo/bar
    -> { root = /.; subpath = "./foo/bar"; }

This is part of the path library effort and needed for the implementation of file set combinators.

This work is sponsored by Antithesis

Things done
  • Tests
  • Docs

@infinisil infinisil requested a review from edolstra as a code owner July 19, 2023 14:23
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Jul 19, 2023
@infinisil infinisil mentioned this pull request Jul 14, 2023
13 tasks
@infinisil infinisil changed the title lib.path.parts: init lib.path.parts: init Jul 19, 2023
@roberth
Copy link
Member

roberth commented Jul 19, 2023

You know I like parts, but I was kind of expecting this to do the splitting between / thing.
I'd frame this as making the path relative or splitting it from its root.
lib.path.splitRoot?

@infinisil infinisil mentioned this pull request Jul 19, 2023
7 tasks
@infinisil infinisil force-pushed the lib.path.parts branch 2 times, most recently from d2acf75 to 137e724 Compare July 19, 2023 15:33
@infinisil
Copy link
Member Author

I like it, applied!

lib/path/default.nix Outdated Show resolved Hide resolved
Split the filesystem root from a path.
The result is an attribute set with these attributes:

- `root`: The filesystem root of the path, meaning that this directory has no parent directory.
Copy link
Member

Choose a reason for hiding this comment

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

It's unfortunate that this concept does not have Nix documentation yet.

(no action required now)

lib/path/default.nix Outdated Show resolved Hide resolved
lib/path/default.nix Show resolved Hide resolved
lib/path/tests/unit.nix Show resolved Hide resolved
@fricklerhandwerk fricklerhandwerk changed the title lib.path.parts: init lib.path.splitRoot: init Jul 19, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jul 20, 2023
@infinisil
Copy link
Member Author

I fixed the bug in nixdoc that removed all indentation: nix-community/nixdoc#62

That was just too annoying when we're trying to have some nice formatting in the docs!

lib/path/default.nix Show resolved Hide resolved
lib/path/default.nix Outdated Show resolved Hide resolved
lib/path/default.nix Outdated Show resolved Hide resolved
=> { root = /.; subpath = "./bar"; }

splitRoot "/foo/bar"
=> <error>
Copy link
Member

Choose a reason for hiding this comment

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

Reader will wonder what's the problem with this? Surely / is the root.

I think it might be sensible to treat all "${storeDir}/${x}" as roots, but that's a discussion for another time.

Copy link
Member Author

Choose a reason for hiding this comment

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

That feels like something that should be supported by lib.strings.* instead. The error could even redirect the user to that

lib/path/default.nix Outdated Show resolved Hide resolved
infinisil and others added 2 commits July 26, 2023 23:20
@infinisil infinisil requested a review from roberth July 26, 2023 21:21
@infinisil
Copy link
Member Author

I think this is ready to be merged

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.

Scoping out per-sublibrary and per-type docs.

@roberth roberth merged commit 399ac29 into NixOS:master Jul 27, 2023
@infinisil infinisil deleted the lib.path.parts branch July 27, 2023 12:56
@infinisil
Copy link
Member Author

Scoping out per-sublibrary and per-type docs.

Not sure what you mean with that, but thanks for the merge :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants