Improve exit status section

[codesections]

As noted in the comments, the exit_status section is currently hard-coded, but should be revised to accept arguments. Specifically, the exit status section will currently always be:

EXIT STATUS
        0      Successful program execution.

        1      Unsuccessful program execution.

        101    The program panicked.

Looking at a variety of man pages, I've noticed that not all of them have an "exit status" section (including some that I trust to be good, including git, bash, zsh, and rg. Thus, I think that we should make the exit status section optional.

Of course, as noted in the comment, we should make it possible to pass app-specific exit codes. However, it seems like the current implementation (0, 1, and 101) will be a very common default, so it would be nice if we keep that as fairly easy behavior.

Given all of the above, I propose the following API

.exit_status(ExitStatus::default())

produces current behavior

.exit_status(ExitStatus::default_plus([(1, "invalid input"), (3, "could not read file")])

Produces an exit status section with 0 and 101 as defaults plus whatever custom codes and messages the user supplies in an vec of tuples.

.exit_status(ExitStatus::custom([(0, "success")]))

Produces a fully custom exit status section with only the user-supplied codes/messages.

I'm happy to implement this API; please let me know if you think we should go in a different direction.

[yoshuawuyts]

Should CLIs always document exit codes?

(including some that I trust to be good, including git, bash, zsh, and rg. Thus, I think that we should make the exit status section optional.

Hmm, I wonder about that. In the case of rg (a rust tool) it will definitely have possible exit codes of 0, 1 and 101 even if they're not documented. I'm leaning more towards it being something that ripgrep's docs are missing, than something people might not want to include. But maybe that's me?

API

With the rest of the APIs every item is added individually. That would more or less translate to this:

let page = Manual::new("basic")
  .exit_status(3, "Invalid config.")
  .exit_status(5, "Could not read file.")
  .render();

Optional Defaults?

Circling back to the question of defaults. I think we're on the same page that we want to have a convenient way to include the defaults, but there's a question if they should be enabled by default. I'm thinking there's two options:

• we enable the default status codes, and allow people to opt-out.
• we disable the default status codes, and allow people to opt-in.

This would roughly translate to two possible APIs:

opt-out of defaults

defaults enabled

let page = Manual::new("basic")
  .exit_status(3, "Invalid config.")
  .exit_status(5, "Could not read file.")
  .render();

defaults disabled

let page = Manual::new("basic")
  .no_default_exit_status()
  .exit_status(0, "Successful program execution.")
  .exit_status(1, "Unsuccessful program execution.")
  .exit_status(3, "Invalid config.")
  .exit_status(5, "Could not read file.")
  .exit_status(101, "Program panicked.")
  .render();

opt-in to defaults

defaults enabled

let page = Manual::new("basic")
  .default_exit_status()
  .exit_status(3, "Invalid config.")
  .exit_status(5, "Could not read file.")
  .render();

defaults disabled

let page = Manual::new("basic")
  .exit_status(0, "Successful program execution.")
  .exit_status(1, "Unsuccessful program execution.")
  .exit_status(3, "Invalid config.")
  .exit_status(5, "Could not read file.")
  .exit_status(101, "Program panicked.")
  .render();

---

I'm not sure what the best option here is. I feel there's arguments for both; I'd be curious to hear which people prefer.

[codesections]

Hmm, interesting thoughts.

One point comes to mind: with most of our methods (.arg, .author, .custom, .env, .flag, and .option), we don't take arguments directly—instead, we take a corresponding struct. If we wanted to follow that here, we could have:

opt-out of defaults

let page = Manual::new("basic")
  .exit_status(ExitStatus::no_defaults())
  .exit_status(
    ExitStatus::new(0)
    .description("Successful program execution.")
  )
  .exit_status(
    ExitStatus::new(1)
    .description("Unsuccessful program execution.")
  )
  .exit_status(
    ExitStatus::new(3)
    .description("Invalid config.")
  )
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .exit_status(
    ExitStatus::new(101).
    description("Program panicked.")
  )
  .render();

or

opt-into defaults

let page = Manual::new("basic")
  .exit_status(ExitStatus::default())
  .exit_status(
    ExitStatus::new(1)
    .description("Unsuccessful program execution.")
  )
  .exit_status(
    ExitStatus::new(3)
    .description("Invalid config.")
  )
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .render();

Admittedly, this version of the API is slightly more verbose—but I think I'd prefer it a bit based on the consistency with the rest of the API. Plus, it seems like it makes the default option a little clearer than having an explicit default or no_default option.

One related point: In the first API I proposed, I was suggesting that the/a default could preserve the current behavior (giving 0, 1, and 101 exit codes). The API you proposed, on the other hand, had only 0 and 101.

If we could have all three as the default, that seems to argue a bit in favor of the opt-in: the most common use-case would just be

let page = Manual::new("basic")
  .exit_status(ExitStatus::default())
  .render();

Of course, the downside of providing more in the default is that more people would need to opt out.

[yoshuawuyts]

with most of our methods (...), we don't take arguments directly—instead, we take a corresponding struct.

@codesections originally those APIs didn't take structs either. But because every argument ended up being on Option<> it felt better to use builders. I think it's worth considering if the status code's description should be optional. It probably should tho?

If we could have all three as the default, that seems to argue a bit in favor of the opt-in: the most common use-case would just be.

I think that example looks pretty reasonable actually -- I like it a lot!

[killercup]

We discussed the API proposed here in #30 and I noticed that I don't fully agree with it. The main issue for me is that .exit_status does two things. See #30 (comment) for more context.

Here's the API I'd try to offer, with my expectations. This is very similar to what @yoshuawuyts proposed as "opt-in to defaults" above.

Empty

let page = Manual::new("basic").render();

This does not output a list of error codes.

Just default errors

let page = Manual::new("basic")
  .use_default_exit_statuses()
  .render();

This renders the three default error codes (with their default description).

The basic case

let page = Manual::new("basic")
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .render();

This renders an exit code section with just one error code listed, 5.

Defaults and more

let page = Manual::new("basic")
  .use_default_exit_statuses()
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .render();

This renders the three default error codes, plus the exit code 5.

Overwrite an exit code

(optional to implement but would make sense to me to support)

let page = Manual::new("basic")
  .use_default_exit_statuses()
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .exit_status(
    ExitStatus::new(5)
    .description("Foobar")
  )
  .render();

This renders the three default error codes, plus the exit code 5 which has "Foobar" s description.

[codesections]

I like this API, and agree that it's similar to one of @yoshuawuyts's suggestions above.

My only concern with this is that it adds two methods to Manual to deal with exit codes. This doesn't clutter the API much but could do so if we want to have other default behaviors that users can opt in to (or out of).

I was reviewing the clap-rs API to see how they handle it, and it looks like they have a settings method.

I wonder if we could do the same? If we did that, here's what the API examples from above would look like:

(Another advantage of pulling this out into a setting is that we could make this (or any other default) opt-out if we later want to)

Empty

let page = Manual::new("basic").render();

This does not output a list of error codes.

Just default errors

let page = Manual::new("basic")
  .setting(ManSetting::DefaultExitStatuses)
  .render();

This renders the three default error codes (with their default description).

The basic case

let page = Manual::new("basic")
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .render();

This renders an exit code section with just one error code listed, 5.

Defaults and more

let page = Manual::new("basic")
  .setting(ManSetting::DefaultExitStatuses)
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .render();

This renders the three default error codes, plus the exit code 5.

Overwrite an exit code

(optional to implement but would make sense to me to support)

let page = Manual::new("basic")
  .setting(ManSetting::DefaultExitStatuses)
  .exit_status(
    ExitStatus::new(5)
    .description("Could not read file")
  )
  .exit_status(
    ExitStatus::new(5)
    .description("Foobar")
  )
  .render();

This renders the three default error codes, plus the exit code 5 which has "Foobar" s description.