Repository navigation
docs(mkdir): the --verbose example showed output that never occurs - #19023
Merged
fdncred merged 1 commit intoSep 14, 2026
Merged
Conversation
`help mkdir` documents `mkdir -v foo/bar foo2` as returning `path: "foo/bar"` and `path: "foo2"`. It does not: each argument goes through `nu_path::expand_path_with` before it is reported, so the real values are absolute paths. The output depends on the working directory, so it cannot be written down as a literal at all. Drop the declared result, as the first example already does, and say in the description that the paths are absolute. `umkdir.rs` has no `test_examples()` — unlike the `path` commands — so nothing checked the declared result against reality. Pin the behavior with an integration test instead: `verbose_reports_the_expanded_path` asserts the reported path equals the expanded one, and fails against the value the example used to claim. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Contributor
|
Thanks |
fdncred
pushed a commit
that referenced
this pull request
Sep 14, 2026
…9024) ## Description `view source` renders a rest parameter without a space after the colon, unlike every other parameter kind: ```nu > def test [a?:any --b:int ...rest:string] { echo 'test' }; view source test def test [ a?: any --b: int ...rest:string] { echo 'test' } ``` `a?: any` and `--b: int` are spaced; `...rest:string` is not. In `render_def` the branches are: | kind | format | | --- | --- | | required positional | `"{}: {} "` | | optional positional | `"{}?: {} "` | | flag with an argument | `": {arg}"` | | rest | `"...{}:{}"` | The rest branch is the only one missing the space. The command's own examples already show `...rest: string`, in two of them, so the documented form was the right one and the code disagreed with it. Why it was not caught: `view_source.rs` declares eight example results and had no test running them. Adding `test_examples` for `ViewSource` is what surfaced this, and it also caught a second, smaller mismatch — the module example claimed ``` " export-env { $env.FOO_ENV = 'BAZ' }" ``` while `view source` returns the source between the module's braces verbatim, so the real value has the trailing space too. That one is the example being wrong rather than the code, so the example is corrected. ## User-facing changes (Release notes) `view source` now renders a rest parameter as `...rest: string` rather than `...rest:string`, matching how it already renders every other parameter. ## Tests + Formatting Added `test_examples` for `ViewSource`, which runs all eight declared example results. It is not vacuous — it is what found both mismatches, failing first on ``` expected: "def --env --wrapped foo [ ...rest: string] { print $rest }" got: "def --env --wrapped foo [ ...rest:string] { print $rest }" ``` and then on the module example. Both are green now. The full `nu-command` lib suite (815 tests) and the existing `view_source` integration tests pass. I checked nothing asserted the old spelling: the only other `...name:type` in the tree is input source in an unrelated parser test, not an assertion on `view source` output. `cargo fmt --all -- --check` and `cargo clippy --all-targets` are clean. ## Additional notes Third small `nu-command` fix from me after #19017 and #19023, same theme: a declared example result that nothing was checking. Co-authored-by: Prajwal Narayana <[email protected]> Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
help mkdirshows this for the--verboseexample:mkdirnever produces that. Every argument goes throughnu_path::expand_path_with(..)before it reaches the output, so the reportedpaths are absolute:
Since the value depends on the working directory, it cannot be written down as
a literal at all — so this drops the declared
result, the way the firstexample in the same command already does, and says in the description that the
paths are absolute.
Why it went unnoticed:
umkdir.rshas notest_examples(), unlike the commandsin
crates/nu-command/src/path/, so the declared result was never comparedagainst what the command actually returns.
User-facing changes (Release notes)
n/a — documentation only. The
help mkdirexample no longer shows output thatthe command cannot produce.
Tests + Formatting
Added
verbose_reports_the_expanded_path, which asserts the reported pathequals the expanded path rather than the argument as typed. It is not vacuous:
pointing it at the value the example used to claim produces
which is the bug this PR fixes. All 18
umkdirtests pass;cargo fmt --all -- --checkandcargo clippy --all-targetsare clean.I did not add
test_examples()for this command: its examples createdirectories as a side effect, which is presumably why it has none.
Additional notes
Follow-up to #19017, same command, unrelated bug.