Clarify ExitStatusExt documentation#158574
Open
schneems wants to merge 1 commit into
Open
Conversation
Collaborator
|
r? @Darksonn rustbot has assigned @Darksonn. Use Why was this reviewer chosen?The reviewer was selected based on:
|
## Fix struct links There are several structs (`ExitStatus` and `ExitStatusError`) that could be linked, but aren't. Fixed. All links show their struct now instead of previously some showed `process::ExitStatus` when rendered. ## Add `wait` links The docs distinguish `wait` as code, but do not link to **what** wait they're referring to. As this is Unix extension documentation, adding a link to the POSIX standard for `wait`. Showing a construction of an `ExitStatus` for both a signal and an exit code helps to highlight and internalize what **wait status, not an exit status** means. ## Example for `from_raw` Current documentation calls out "The value should be a **wait status, not an exit status**." but it's unclear what exactly that means without a reference or an example. I added a doctest that shows you need to shift by 8 to get the desired result and annotated it with information about how that's derived based on the linked `wait` documentation. ## Example to `ExitStatus::signal` Called out that a signaled process does not also produce an exit code. Added an example. Called out that just because a process reports that it was not terminated via a signal doesn't mean it never got one (it could be handled). Also added a note correlating shell behavior to exit codes: GNU Bash manual: https://web.archive.org/web/20260625050034/https://www.gnu.org/software/bash/manual/bash.html#Exit-Status-1 > [...] a fatal signal whose number is N, Bash uses the value 128+N as the exit status. FreeBSD bash(1) man page: https://man.freebsd.org/bash/1 > The return value [...] 128+n if the command is terminated by signal n. ## Panic scope on `from_raw` This change is based on preference/experience: When I originally read this documentation, it was because I clicked through `Output` -> `ExitStatus`. While the text "Creates a new `ExitStatus` or `ExitStatusError`" is present at the top, I didn't internalize what that meant exactly. Therefore, when I got to the panic section, I was mildly confused. This edit makes the behavior more explicitly attached to the type. While the original format had all of the same information available, it was left for the reader to bridge that "making an ExitStatusError" is referring to `ExitStatus::from_raw` as that's the only constructor (for now). Making this bridge more explicit also future-proofs us for future constructors (if they're ever added) that may or may not panic. While the context of the panic documentation is directly attached to `from_raw`, it's slightly (pedantically) more correct to not imply that ANY way of creating an ExitStatusError` from a `wait` of `0` will panic (a different constructor could perhaps return an option instead). Technically, the syntax isn't correct, as it should be `<ExitStatusError as ExitStatusExt>::from_raw`, but I think this conveys intent without being overly verbose. Co-authored-by: Colin Casey <casey.colin@gmail.com>
8b1de0a to
b26864a
Compare
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.
Fix struct links
There are several structs (
ExitStatusandExitStatusError) that could be linked, but aren't. Fixed. All links show their struct now instead of previously some showedprocess::ExitStatuswhen rendered.Add
waitlinksThe docs distinguish
waitas code, but do not link to what wait they're referring to. As this is Unix extension documentation, adding a link to the POSIX standard forwait. Showing a construction of anExitStatusfor both a signal and an exit code helps to highlight and internalize what wait status, not an exit status means.Example for
from_rawCurrent documentation calls out "The value should be a wait status, not an exit status." but it's unclear what exactly that means without a reference or an example. I added a doctest that shows you need to shift by 8 to get the desired result and annotated it with information about how that's derived based on the linked
waitdocumentation.Example to
ExitStatus::signalCalled out that a signaled process does not also produce an exit code. Added an example. Called out that just because a process reports that it was not terminated via a signal doesn't mean it never got one (it could be handled). Also added a note correlating shell behavior to exit codes:
GNU Bash manual: https://web.archive.org/web/20260625050034/https://www.gnu.org/software/bash/manual/bash.html#Exit-Status-1
FreeBSD bash(1) man page: https://man.freebsd.org/bash/1
Panic scope on
from_rawThis change is based on preference/experience:
When I originally read this documentation, it was because I clicked through
Output->ExitStatus. While the text "Creates a newExitStatusorExitStatusError" is present at the top, I didn't internalize what that meant exactly. Therefore, when I got to the panic section, I was mildly confused.This edit makes the behavior more explicitly attached to the type. While the original format had all of the same information available, it was left for the reader to bridge that "making an ExitStatusError" is referring to
ExitStatus::from_rawas that's the only constructor (for now).Making this bridge more explicit also future-proofs us for future constructors (if they're ever added) that may or may not panic. While the context of the panic documentation is directly attached to
from_raw, it's slightly (pedantically) more correct to not imply that ANY way of creating an ExitStatusErrorfrom awaitof0` will panic (a different constructor could perhaps return an option instead).Technically, the syntax isn't correct, as it should be
<ExitStatusError as ExitStatusExt>::from_raw, but I think this conveys intent without being overly verbose.