FEATURE: print example command results in the `help`

[amtoine]

Should close #8035.

Note this is my first technical PR for nushell

• i might very well miss things
• i tried to be as complete as possible about the changes
• please require further changes if i did something wrong, i'm open to any remark :relieved:

Description

this PR adds, when it is defined in the examples method of the Command implementations, the output of the examples to the output of the help command.

this PR

• only modifies crates/nu-engine/src/documentation.rs and the get_documentation function
• defines a new WD constant to print a White Dimmed ...
• a match statement at the end of the example loop to
  • print a white dimmed ... when the example is not set, i.e. set to None in the examples method of the Command implementation of a command
  • pretty print the output of the associated example Value when it has been defined

Warning LIMITATIONS:

• i use snippets from crates/nu-protocol/src/pipeline_data.rs
  • the table creation from pub PipelineData::print, i.e. the let decl_id = ...; and let table = ...; in the change
  • the table item printing from PipelineData::write_all_and_flush, i.e. the for item in table { ... }

ADDRESSED:

• ~~the formatting of the output is not perfect and has to be fully left aligned with the first column for now~~ (fixed with 5abeefd558c34ba9bae15e2f183ff4625442921e..a62be1b5a2c730959da5dbc028bb91ffe5093f63)
• ~~i'm using .unwrap() on both the changes above, not sure how to handle this for now~~ (fixed for now thanks to 49f1dc080)
• ~~the tests and clippy checks do not pass for now, see below~~ (clippy now is happy with 49f1dc080 and the tests pass with 11666bc71526b31a27105a362ce11cf94a55b8b6)

User-Facing Changes

the output of the help <command> command is now augmented with the outputs of the examples, when they are defined.

• with-env

> help with-env
...
Examples:
  Set the MYENV environment variable
  > with-env [MYENV "my env value"] { $env.MYENV }
  my env value

  Set by primitive value list
  > with-env [X Y W Z] { $env.X }
  Y

  Set by single row table
  > with-env [[X W]; [Y Z]] { $env.W }
  Z

  Set by key-value record
  > with-env {X: "Y", W: "Z"} { [$env.X $env.W] }
  ╭───┬───╮
  │ 0 │ Y │
  │ 1 │ Z │
  ╰───┴───╯

instead of the previous

> help with-env
...
Examples:
  Set the MYENV environment variable
  > with-env [MYENV "my env value"] { $env.MYENV }

  Set by primitive value list
  > with-env [X Y W Z] { $env.X }

  Set by single row table
  > with-env [[X W]; [Y Z]] { $env.W }

  Set by key-value record
  > with-env {X: "Y", W: "Z"} { [$env.X $env.W] }

• merge

> help merge
...
Examples:
  Add an 'index' column to the input table
  > [a b c] | wrap name | merge ( [1 2 3] | wrap index )
  ╭───┬──────╮
  │ # │ name │
  ├───┼──────┤
  │ 1 │ a    │
  │ 2 │ b    │
  │ 3 │ c    │
  ╰───┴──────╯

  Merge two records
  > {a: 1, b: 2} | merge {c: 3}
  ╭───┬───╮
  │ a │ 1 │
  │ b │ 2 │
  │ c │ 3 │
  ╰───┴───╯

  Merge two tables, overwriting overlapping columns
  > [{columnA: A0 columnB: B0}] | merge [{columnA: 'A0*'}]
  ╭───┬─────────┬─────────╮
  │ # │ columnA │ columnB │
  ├───┼─────────┼─────────┤
  │ 0 │ A0*     │ B0      │
  ╰───┴─────────┴─────────╯

instead of the previous

> help merge
...
Examples:
  Add an 'index' column to the input table
  > [a b c] | wrap name | merge ( [1 2 3] | wrap index )

  Merge two records
  > {a: 1, b: 2} | merge {c: 3}

  Merge two tables, overwriting overlapping columns
  > [{columnA: A0 columnB: B0}] | merge [{columnA: 'A0*'}]

Tests + Formatting

no new tests have been added, as there is no real functionality to be tested, only previously existing pieces put together in the documentation.rs

• :heavy_check_mark: cargo fmt --all
• :heavy_check_mark: cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect (passes thanks to 49f1dc0)
• :heavy_check_mark: cargo test --workspace (passes thanks to 11666bc71526b31a27105a362ce11cf94a55b8b6)

After Submitting

Warning maybe that would require generating the web pages of the commands again? :thinking:

[amtoine]

main comment modified with 49f1dc080 :yum:

[amtoine]

and now the tests should be fixed with 11666bc71526b31a27105a362ce11cf94a55b8b6 :+1:

[amtoine]

the indentation should now be fixed with 5abeefd558c34ba9bae15e2f183ff4625442921e..a62be1b5a2c730959da5dbc028bb91ffe5093f63 :relieved:

[codecov[bot]]

Codecov Report

Merging #8189 (a62be1b) into main (e89866b) will increase coverage by 0.05%. The diff coverage is 88.88%.

:exclamation: Current head a62be1b differs from pull request most recent head bddc259. Consider uploading reports for the commit bddc259 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8189      +/-   ##
==========================================
+ Coverage   54.23%   54.28%   +0.05%     
==========================================
  Files         608      612       +4     
  Lines       99012    99792     +780     
==========================================
+ Hits        53702    54177     +475     
- Misses      45310    45615     +305     

Impacted Files	Coverage Δ
crates/nu-engine/src/documentation.rs	65.28% <88.88%> (+2.67%)	:arrow_up:
crates/nu-protocol/src/ty.rs	73.57% <0.00%> (-13.04%)	:arrow_down:
crates/nu-command/src/filesystem/cp.rs	14.42% <0.00%> (-3.04%)	:arrow_down:
crates/nu-command/src/platform/input.rs	28.05% <0.00%> (-2.18%)	:arrow_down:
crates/nu-protocol/src/syntax_shape.rs	69.76% <0.00%> (-1.17%)	:arrow_down:
crates/nu-command/src/core_commands/let_.rs	98.87% <0.00%> (-1.13%)	:arrow_down:
crates/nu-parser/src/known_external.rs	55.44% <0.00%> (-1.00%)	:arrow_down:
crates/nu-command/src/conversions/into/string.rs	77.82% <0.00%> (-0.32%)	:arrow_down:
crates/nu-parser/src/parse_keywords.rs	40.23% <0.00%> (-0.10%)	:arrow_down:
crates/nu-parser/src/lex.rs	75.96% <0.00%> (-0.08%)	:arrow_down:
... and 26 more

[amtoine]

and now that the tests all pass and the output is as expected originally, i'll mark this a "ready for review" :wink:

[fdncred]

I like the output. I think it looks cool! Good job. Let's see what others think.

[amtoine]

I like the output. I think it looks cool! Good job.

oooh cool amazing :star_struck: glad you like that, i think it makes the help much more ... helpful :yum:

(i love examples...)

Let's see what others think.

of course, cant't wait :relieved:

[rgwood]

Very cool, congrats on your first technical PR!

I'm not sure about the ... for examples without an expected result:

That feels a little unnecessary, especially for commands where every result is .... Maybe we should just output nothing instead of ...?

[amtoine]

Very cool, congrats on your first technical PR!

:blush:

I'm not sure about the ... for examples without an expected result:

...

That feels a little unnecessary, especially for commands where every result is .... Maybe we should just output nothing instead of ...?

actually that mainly was to make sure the changes did indeed apply to all the examples, even the ones without any output. this is why i did print them with {WB}, not to see them too much

i see two options

• remove them altogether
• replace the ... by something still subtle but more explicit ? :thinking:

[fdncred]

I kind of like the ... because it shows which examples are not testable. i.e. they have result: None in the example definition. I fixed one of those today already.

I'm not saying we should necessarily keep the ... but I like the idea of somehow identifying which examples are not testable. Some examples are not testable for a reason and some are not testable because someone was lazy (probably me). ;)

[rgwood]

I would vote to remove the ... altogether. Still pretty easy to check which examples aren't tested.

[sophiajt]

I do like showing the results. I agree with @rgwood that for examples without output, we shouldn't show anything.

Once we have this, we should also watch to make sure the sample output is small and readable, so we don't show any large tables 😅

[amtoine]

thanks guys for the feedback :yum: i'll remove them!

[amtoine]

:bulb: i'm thinking about something :smirk:

• do not print anything when the result of an Example is set to None
• print something when the output is empty, e.g. [] for an empty list or "" for an empty string

the idea is to avoid confusion between "no output" and "an empty output" and still being able to pin down what outputs are "not testable".

[amtoine]

@jntrnr

Once we have this, we should also watch to make sure the sample output is small and readable, so we don't show any large tables sweat_smile

i can do that after this PR :wink:

[amtoine]

... removed in a62be1b5a2c730959da5dbc028bb91ffe5093f63..bddc2595a61e5a6c18196214fc77a88a5f6de71a

[rgwood]

Thanks to both of you for getting this over the line. This is a nice UX improvement 📈

[fdncred]

yay, this landed!!! so excited to play with it. great job @amtoine!! and thanks @sholderbach for helping!

[fdncred]

Did the plugin examples get eaten by this PR? or was it another PR? @WindSoilder just added PluginExamples and help query web used to show them. Now it doesn't.

[sholderbach]

Did the plugin examples get eaten by this PR? or was it another PR? @WindSoilder just added PluginExamples and help query web used to show them. Now it doesn't.

Based on the code it shouldn't as this looks purely additively. Should just add a result at the end of each example to print.

[fdncred]

I was thinking the same thing. It must of been another PR that broke it. Thanks.

[amtoine]

i'm glad it finally landed :tada:

thanks for your kind words, this first technical PR was a great deal of fun :yum: