Skip to content

FIX: have consistent errors between std help and std help ...#9101

Merged
fdncred merged 1 commit intonushell:mainfrom
amtoine:feature/throw-error-when-nothing-matches-item-in-std-help
May 8, 2023
Merged

FIX: have consistent errors between std help and std help ...#9101
fdncred merged 1 commit intonushell:mainfrom
amtoine:feature/throw-error-when-nothing-matches-item-in-std-help

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented May 4, 2023

Description

an example should show what happens quite clearly 😋

Note
the ugly spanned errors you'll see below are fixed in #9039 😌

in all cases we get

> std help commands does-not-exist-anywhere
Help pages from external command does-not-exist-anywhere:
No manual entry for does-not-exist-anywhere
Error:
  × std::help::command_not_found
     ╭─[help:662:1]
 662 │
 663 │     let command = ($command | str join " ")
     ·     ─┬─
     ·      ╰── command not found
 664 │
     ╰────

but

❌ before this PR

> std help does-not-exist-anywhere
Help pages from external command does-not-exist-anywhere:
No manual entry for does-not-exist-anywhere

without any error, which makes it inconsistent with all the other std help commands which give errors when not finding an item 🤔

✔️ with this PR

> std help does-not-exist-anywhere
Help pages from external command does-not-exist-anywhere:
No manual entry for does-not-exist-anywhere
Error:
  × std::help::item_not_found
     ╭─[help:740:1]
 740 │
 741 │     let item = ($item | str join " ")
     ·     ─┬─
     ·      ╰── item not found
 742 │
     ╰────

User-Facing Changes

more consistent errors when using std help and std help commands, as shown above.

Tests + Formatting

After Submitting

$nothing

@amtoine amtoine added the A:std-library Defining and improving the standard library written in Nu label May 4, 2023
@amtoine amtoine self-assigned this May 4, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented May 4, 2023

Codecov Report

Merging #9101 (94a14b0) into main (1acc2bf) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9101      +/-   ##
==========================================
- Coverage   68.71%   68.71%   -0.01%     
==========================================
  Files         640      640              
  Lines      101681   101681              
==========================================
- Hits        69873    69866       -7     
- Misses      31808    31815       +7     

see 3 files with indirect coverage changes

@bobhy
Copy link
Copy Markdown
Contributor

bobhy commented May 5, 2023

But wouldn't it be better if the error could point to the span containing the actually-not-found command?
e.g:

〉help commands gargle
Help pages from external command gargle:
No manual entry for gargle
Error: 
  × std::help::command_not_found
   ╭─[entry #205:1:1]
 1 │ help commands gargle
   ·               ───┬──
   ·                  ╰── command not found
   ╰────

You can do this by avoiding shadowing parameter command in the body of help commands (likewise in the other exported defs), like this:

export def "help commands" [
    ...command: string@"nu-complete list-commands"  # the name of command to get help on
    --find (-f): string  # string to find in command names and usage
] {
    let commands = ($nu.scope.commands | where not is_extern | reject is_extern | sort-by name )

    let target = ($command | str join " ")  # don't shadow `command` parameter, we want to use it later...

    if not ($find | is-empty) {
        # TODO: impl find for external commands
        let found_commands = ($commands | find $find --columns [name usage search_terms])

        if ($found_commands | length) == 1 {
            show-command ($found_commands | get 0)
        } else {
            $found_commands | select name category usage signatures search_terms
        }
    } else if not ($command | is-empty) {
        let found_commands = ($commands | where name == $target)

        if not ($found_commands | is-empty) {
            show-command ($found_commands | get 0)
        } else {
            try {
                print $"(ansi default_italic)Help pages from external command ($target | pretty-cmd):(ansi reset)"
                 ^($env.NU_HELPER? | default "man") $command
            } catch {
                command-not-found-error (metadata $command | get span)
            }
        }

    } else {
        $commands | select name category usage signatures search_terms
    }
}

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 5, 2023

@bobhy
as said in the PR description, this is being taken care of in #9039 😌

@bobhy
Copy link
Copy Markdown
Contributor

bobhy commented May 5, 2023

Ah, missed that. Thanks for fixing it!

@fdncred fdncred merged commit 47f9fd3 into nushell:main May 8, 2023
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented May 8, 2023

thanks

@amtoine amtoine deleted the feature/throw-error-when-nothing-matches-item-in-std-help branch May 9, 2023 16:05
amtoine added a commit that referenced this pull request May 12, 2023
related to 
- #9101
- #9039

# Description
i actually forgot to fix in #9039 the new "*item*" introduced by
#9101... 👀
there it is 😇 

# User-facing changes
going from
```
> std help git-commiteuwqi
Help pages from external command git-commiteuwqi:
No manual entry for git-commiteuwqi
Error:
  × std::help::item_not_found
     ╭─[help:721:1]
 721 │
 722 │     let item = ($item | str join " ")
     ·     ─┬─
     ·      ╰── item not found
 723 │
     ╰────
```
to
```
> std help git-commiteuwqi
Help pages from external command git-commiteuwqi:
No manual entry for git-commiteuwqi
Error:
  × std::help::item_not_found
   ╭─[entry #2:1:1]
 1 │ std help git-commiteuwqi
   ·          ───────┬───────
   ·                 ╰── item not found
   ╰────
```

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- ⚫ `toolkit test`
- ⚫ `toolkit test stdlib`

# After Submitting
```
$nothing
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:std-library Defining and improving the standard library written in Nu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants