Skip to content

Path Command Enhancement Project #2742

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

Merged
merged 28 commits into from
Nov 24, 2020
Merged

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Nov 9, 2020

Description

The aim of this PR is to add a couple of options enhancing the capabilities of the path subcommands. More specifically, it makes it easier to mass-rename files and their locations without an excessive use of build-string chains. I found both nu and traditional Unix tools (such as dirname or basename) too verbose when I was encoding and moving tens of thousands images in tens of different folders all over the place.

Changes

path basename

  • Add --replace <string> option to replace the basename instead of returning it
    • (e.g., echo foo/bar/baz.txt | path basename -r spam.md would return foo/bar/spam.md)
  • Examples

path dirname

  • Add -n <int> option to specify how many levels up to return
    • (e.g., echo foo/bar/baz.txt | path dirname -n 2 would return foo)
  • Add --replace <string> option to replace the dir instead of returning it (respecting the -n option)
    • (e.g., echo foo/bar/baz.txt | path dirname -n 2 -r eggs would return eggs)
  • Examples
  • Handle num_levels < 0 properly -- current message is good enough

path extension

  • Add --replace <string> option to replace the extension instead of returning it
    • (e.g., echo test.txt | path extension -r md would return test.md)
  • Examples

path filestem

  • Add --replace <string> option to replace the filestem instead of returning it
    • (e.g., echo foo/bar/baz.txt | path basename -r spam would return foo/bar/spam.txt)
  • Add an option to spell out the suffix literally to cover cases like .tar.gz
  • Add an option to spell out the prefix literally to cover cases like out_<whatever>
  • Examples
  • Review for possible cleanup

misc

  • Add integration tests to cover edge cases
  • Go through descriptions and examples and make sure they're consistent
  • Replace ...args: optionally operate by path with ...args: optionally operate by column path in subcommands desctiprions
  • Fix failing tests on Windows
    • solution: generate different set of examples on Windows
  • Fix broken expand on Windows -- the command is a bit problematic but I'll leave that for further discussion/PR
  • Return path from subcommands when appropriate (e.g., expand, --replace options)

@fdncred
Copy link
Contributor

fdncred commented Nov 9, 2020

These sounds cool! Thanks!

@sophiajt
Copy link
Contributor

Seconded! I like these

@kubouch kubouch force-pushed the path-enhancement branch 3 times, most recently from d39c754 to e369821 Compare November 21, 2020 16:31
This adds a lot of redundancy to non-relevant subcommands such as type,
exists or expand.
`path expand` is still broken but otherwise seems to fix all examples
on Windows
Also disable example tests for path expand. Failed caconicalization
(e.g., due to path not existing) returns the original path so the
examples always fail.
Wouldn't run on minimal due to useing optional dependency.
The test success was also deending on the presence of home dir on the
testing machine which might not be completely robust.
@kubouch kubouch marked this pull request as ready for review November 22, 2020 15:49
@kubouch
Copy link
Contributor Author

kubouch commented Nov 22, 2020

Alright, path handling turned out to be a little rabbit hole. The commands mostly respect the std::path's way of resolving paths which is not very intuitive at times (for example, . is automatically resolved while .. is not). Also, there are some limitations in the path expand command: most notably, the ~ resolves as home only with / separator but not \ which can lead to strange bugs.

I think the path handling would deserve some consistency revamp so it behaves the same way on all platforms but I'll leave it for a further discussion / PR.

@sophiajt
Copy link
Contributor

This is great, thanks for putting it together!

@sophiajt
Copy link
Contributor

and agreed, we'll want to circle back and improve the path handling a bit more. We can do that in a future PR and land this one, as I think these improvements stand on their own.

@sophiajt sophiajt merged commit 8b59718 into nushell:main Nov 24, 2020
@kubouch kubouch deleted the path-enhancement branch January 19, 2021 20:04
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.

3 participants