feat: Return help message as String for Command #3874
Conversation
ychiguer
force-pushed
the
master
branch
2 times, most recently
from
a3087d3
to
50e5ea7
Compare
| @@ -1117,6 +1121,7 @@ const TAB_WIDTH: usize = 4; | |||
| pub(crate) enum HelpWriter<'writer> { | |||
| Normal(&'writer mut dyn Write), | |||
| Buffer(&'writer mut Colorizer), | |||
| String(&'writer mut String), | |||
There was a problem hiding this comment.
This was to simplify writing to a String, but from the points you have made it shouldn't be necessary.
| Err(e) => { | ||
| panic!("{}", e.to_string()) | ||
| } |
There was a problem hiding this comment.
We should either be returning an error or making it clear why a panic is unexpected to happen. The latter is the case for this. We have a common panic message for these cases (its INTERNAL something I think). We can just write_help().expect(INTERNAL_...)
| @@ -708,6 +708,52 @@ impl<'help> App<'help> { | |||
| c.print() | |||
| } | |||
|
|
|||
| /// Returns the help message (`-h`). | |||
There was a problem hiding this comment.
Please move these functions to a consistent location (we currently have short/long help grouped and show print followed by write variants).
| static SETUP_HELP_MESSAGE: &str = "test 1.3 | ||
| Kevin K. | ||
| tests stuff | ||
|
|
||
| USAGE: | ||
| test | ||
|
|
||
| OPTIONS: | ||
| -h, --help Print help information | ||
| -V, --version Print version information | ||
| "; | ||
|
|
||
| static SETUP_LONG_HELP_MESSAGE: &str = "test 1.3 | ||
| Kevin K. | ||
| tests stuff | ||
|
|
||
| USAGE: | ||
| test | ||
|
|
||
| OPTIONS: | ||
| -h, --help | ||
| Print help information | ||
|
|
||
| -V, --version | ||
| Print version information | ||
| "; |
There was a problem hiding this comment.
Nit; could you move these into the test function bodies?
A lot of older tests do this and the separation between the expected output and the rest of the test function makes it harder to follow. We've been writing new tests with the expected string in the body and updating the old tests piecemeal.
The writer is less convenient and isn't offering any performance benefits of avoidign the extra allocations, so let's render instead. This supersedes clap-rs#3874 Fixes clap-rs#3873
The writer is less convenient and isn't offering any performance benefits of avoidign the extra allocations, so let's render instead. This supersedes clap-rs#3874 Fixes clap-rs#3873
|
#4248 supersedes this |
Fixes #3873