-
Notifications
You must be signed in to change notification settings - Fork 480
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
Replace Pipelines "Behind the Scenes" with "Result Display Rendering" #1635
base: main
Are you sure you want to change the base?
Conversation
This looks like a good, straightforward simplification to me - Thanks! |
As noted in nushell/nushell#14361, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@132ikl Does that suggestion look better?
In interactive mode, when a pipeline ends, the [`display_output` hook configuration](https://www.nushell.sh/book/hooks.html#changing-how-output-is-displayed) defines how the result will be displayed. | ||
The default configuration uses the [`table` command](/commands/docs/table.md) to render structured data as a visual table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In interactive mode, when a pipeline ends, the [`display_output` hook configuration](https://www.nushell.sh/book/hooks.html#changing-how-output-is-displayed) defines how the result will be displayed. | |
The default configuration uses the [`table` command](/commands/docs/table.md) to render structured data as a visual table. | |
In interactive mode, two additional expressions are added to the end of each pipeline to ensure proper rendering of the result: | |
1. First, if defined, the [`display_output` hook](https://www.nushell.sh/book/hooks.html#changing-how-output-is-displayed) is added. This allows the user to change the rendering. | |
2. Nushell then internally adds a `table` command to render the final result. |
You're probably right - When I look at it in light of that, it may be an over-simplification. Does my suggested edit about look more accurate? |
@132ikl thank you for raising that concern. Although I tested with null or empty setting the hook, I assumed Nushell fell back to Shall this PR wait for the discussion in nushell/nushell#14361 then, or make adjustments like @NotTheDr01ds suggested so the improvements can land? I don't know how long or not the referenced discussion may take? |
@Kissaki nushell/nushell#14361 was landed, so I think your original explanation is good. Technically, if the hook is not set (or set to null), the It could be useful to show what happens if you set |
The previous "Behind the Scenes" docs has some problems: * The title is not descriptive * Confusing context reference: The "You may have wondered" references `ls` output despite no applicable example in front of it - and is not stable against other docs section changes either way * Confusing false equivalence: Claims "one and the same" and then explains how they are not in fact the same The section is being replaced with a "Result Display Rendering", which * Mentions and explains the existence of a display output hook * Begins with establishing context: The section is about interactive shell pipeline result display output --- I kept the section small as not to duplicate content from the Hooks documentation page. Adding examples for current, different, or simplified displaying may be reasonable here, but would duplicate the hooks "Changing how Output is Displayed" and "Filtering or Diverting Command Output" sections.
Matching link elsewhere in the file
Co-authored-by: Douglas <32344964+NotTheDr01ds@users.noreply.github.com>
a851b48
to
cfe9430
Compare
It seemed to me like there was nothing open here but nothing happened anymore. Anyway, I applied the suggested headline change, and added examples including behavior with an empty closure as suggested. PTAL |
The previous "Behind the Scenes" docs has some problems:
ls
output despite no applicable example in front of it - and is not stable against other docs section changes either wayThe section is being replaced with "Result Display Rendering", which
I kept the section small so as not to duplicate content from the Hooks documentation page.
Adding examples for current, different, or simplified displaying may be reasonable here, but would duplicate the hooks "Changing how Output is Displayed" and "Filtering or Diverting Command Output" sections.