Skip to content

FIX: correct bad span in std help errors#9039

Merged
amtoine merged 5 commits intonushell:mainfrom
amtoine:fix/stdlib/help-errors
May 12, 2023
Merged

FIX: correct bad span in std help errors#9039
amtoine merged 5 commits intonushell:mainfrom
amtoine:fix/stdlib/help-errors

Conversation

@amtoine
Copy link
Copy Markdown
Member

@amtoine amtoine commented Apr 28, 2023

Description

❌ before this PR

>_ std help modules euwioq
Error: nu::shell::external_command

  × External command failed
     ╭─[help:259:1]
 259 │         if ($found_module | is-empty) {
 260 │             module_not_found_error (metadata $module | get span)
     ·                                    ──────────────┬──────────────
     ·                                                  ╰── Cannot convert record<start: int, end: int> to a string
 261 │         }
     ╰────
  help: All arguments to an external command need to be string-compatible
>_ std help externs euwioq
Error:
  × std::help::extern_not_found
     ╭─[help:401:1]
 401 │
 402 │     let extern = ($extern | str join " ")
     ·     ─┬─
     ·      ╰── extern not found
 403 │
     ╰────

Note
same kind of error with all the others

✔️ after this PR

> std help modules euwioq                                                                                         04/28/2023 05:45:50 PM
Error:
  × std::help::module_not_found
   ╭─[entry #2:1:1]
 1 │ std help modules euwioq
   ·                  ───┬──
   ·                     ╰── module not found
   ╰────
> std help externs euwioq                                                                                         04/28/2023 05:45:53 PM
Error:
  × std::help::extern_not_found
   ╭─[entry #3:1:1]
 1 │ std help externs euwioq
   ·                  ───┬──
   ·                     ╰── extern not found
   ╰────

Note
same with the others

User-Facing Changes

fixes the errors to have proper messages

Tests + Formatting

  • 🟢 toolkit fmt
  • 🟢 toolkit clippy
  • toolkit test
  • toolkit test stdlib

After Submitting

$nothing

@amtoine amtoine added category:bug Something isn't working A:std-library Defining and improving the standard library written in Nu labels Apr 28, 2023
@amtoine amtoine self-assigned this Apr 28, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 28, 2023

Codecov Report

Merging #9039 (64923de) into main (43a3983) will decrease coverage by 0.19%.
The diff coverage is n/a.

❗ Current head 64923de differs from pull request most recent head 5fdb2bf. Consider uploading reports for the commit 5fdb2bf to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9039      +/-   ##
==========================================
- Coverage   68.90%   68.71%   -0.19%     
==========================================
  Files         641      640       -1     
  Lines      102337   101681     -656     
==========================================
- Hits        70516    69875     -641     
+ Misses      31821    31806      -15     

see 25 files with indirect coverage changes

@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 4, 2023

solving the conflicts... ♻️

This should solve the merge conflicts.
@amtoine amtoine changed the title FIX: correct errors in std help errors FIX: correct bad span in std help errors May 4, 2023
This avoids seeing `[foo, bar, baz]` when running
```
std help foo bar baz
```
This commit makes it `foo bar baz`.
@amtoine
Copy link
Copy Markdown
Member Author

amtoine commented May 5, 2023

adding an idea from #9101
cc/ @bobhy

the only thing is that i still get an incorrect span 🤔

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

i tried to apply

diff --git a/crates/nu-std/lib/help.nu b/crates/nu-std/lib/help.nu
index b33594ee7..cb6ee144b 100644
--- a/crates/nu-std/lib/help.nu
+++ b/crates/nu-std/lib/help.nu
@@ -672,7 +672,10 @@ export def "help commands" [
                 print $"(ansi default_italic)Help pages from external command ($target_command | pretty-cmd):(ansi reset)"
                  ^($env.NU_HELPER? | default "man") $target_command
             } catch {
-                command-not-found-error (metadata $command | get span)
+                command-not-found-error {
+                    start: (metadata ($command | get 0) | get span.start)
+                    end: (metadata ($command | last 1 |get 0) | get span.end)
+                }
             }
         }

but i get the following now 🤔

Error: 
  × std::help::command_not_found
     ╭─[help:675:1]
 675 │                     command-not-found-error {
 676 │ ╭─▶                     start: (metadata ($command | get 0) | get span.start)
 677 │ ├─▶                     end: (metadata ($command | last 1 |get 0) | get span.end)
     · ╰──── command not found
 678 │                     }
     ╰────

@bobhy
Copy link
Copy Markdown
Contributor

bobhy commented May 6, 2023 via email

fdncred pushed a commit that referenced this pull request May 8, 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
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
```
$nothing
```
This commit should fix the merge conflicts in !9039.
@amtoine amtoine merged commit 9ebb61f into nushell:main May 12, 2023
@amtoine amtoine deleted the fix/stdlib/help-errors branch May 12, 2023 17:44
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 category:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants