Skip to content
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

feat(mangen): clap/env is itself a feature of mangen #3729

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions clap_mangen/Cargo.toml
Expand Up @@ -36,15 +36,21 @@ bench = false

[dependencies]
roff = "0.2.1"
clap = { path = "../", version = "3.1.10", default-features = false, features = ["std", "env"] }
clap = { path = "../", version = "3.1.10", default-features = false, features = ["std"] }

[dev-dependencies]
snapbox = { version = "0.2", features = ["diff"] }
clap = { path = "../", version = "3.1.10", default-features = false, features = ["std"] }

[features]
default = []
default = ["env"]

debug = ["clap/debug"]
env = ["clap/env"]

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

[[example]]
name = "man"
required-features = ["env"]
7 changes: 7 additions & 0 deletions clap_mangen/README.md
Expand Up @@ -51,3 +51,10 @@ fn main() -> std::io::Result<()> {
```

Tip: Consider a [cargo xtask](https://github.com/matklad/cargo-xtask) instead of a `build.rs` to reduce build costs.


## Feature Flags

#### Optional features

* **env**: Enable Clap's `env` feature.
3 changes: 3 additions & 0 deletions clap_mangen/src/render.rs
Expand Up @@ -116,6 +116,7 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
roff.text(header);
roff.text(body);

#[cfg(feature = "env")]
if let Some(env) = option_environment(opt) {
roff.control("RS", []);
roff.text(env);
Expand Down Expand Up @@ -147,6 +148,7 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
roff.text(header);
roff.text(body);

#[cfg(feature = "env")]
if let Some(env) = option_environment(pos) {
roff.control("RS", []);
roff.text(env);
Expand Down Expand Up @@ -212,6 +214,7 @@ fn long_option(opt: &str) -> Inline {
bold(&format!("--{}", opt))
}

#[cfg(feature = "env")]
fn option_environment(opt: &clap::Arg) -> Option<Vec<Inline>> {
Comment on lines +217 to 218
Copy link
Member

@epage epage May 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had missed this when fixing some of the other feature flags. In my opinion, stable but optional features should have no-op reflection functions when disabled, so instead we'd offer a version of get_env that returns None and is_hide_env_set returns true when env is not set. That'd make it so clap_mangen needs to do nothing extra.

I'm also tempted to just enable it. It doesn't really buy us much in terms of compile times or binary size and as we modularize, it'll shifting to being optional but without feature flags which aren't ideal to work with.

if opt.is_hide_env_set() {
return None;
Expand Down
21 changes: 14 additions & 7 deletions clap_mangen/tests/common.rs
Expand Up @@ -232,14 +232,21 @@ pub fn hidden_option_command(name: &'static str) -> clap::Command<'static> {
}

pub fn env_value_command(name: &'static str) -> clap::Command<'static> {
let arg = clap::Arg::new("config")
.short('c')
.long_help("Set configuration file path")
.required(false)
.takes_value(true)
.default_value("config.toml");
clap::Command::new(name).arg(
clap::Arg::new("config")
.short('c')
.long_help("Set configuration file path")
.required(false)
.takes_value(true)
.default_value("config.toml")
.env("CONFIG_FILE"),
#[cfg(feature = "env")]
{
arg.env("CONFIG_FILE")
},
#[cfg(not(feature = "env"))]
{
arg
},
)
}

Expand Down
9 changes: 9 additions & 0 deletions clap_mangen/tests/roff.rs
Expand Up @@ -56,6 +56,15 @@ fn hidden_options() {
common::assert_matches_path("tests/snapshots/hidden_option.bash.roff", cmd);
}

#[cfg(not(feature = "env"))]
#[test]
fn value_no_env() {
let name = "my-app";
let cmd = common::env_value_command(name);
common::assert_matches_path("tests/snapshots/value_no_env.bash.roff", cmd);
}

#[cfg(feature = "env")]
#[test]
fn value_env() {
let name = "my-app";
Expand Down
15 changes: 15 additions & 0 deletions clap_mangen/tests/snapshots/value_no_env.bash.roff
@@ -0,0 +1,15 @@
.ie /n(.g .ds Aq /(aq
.el .ds Aq '
.TH my-app 1 "my-app "
.SH NAME
my/-app
.SH SYNOPSIS
/fBmy/-app/fR [/fB/-h/fR|/fB/-/-help/fR] [/fB/-c /fR]
.SH DESCRIPTION
.SH OPTIONS
.TP
/fB/-h/fR, /fB/-/-help/fR
Print help information
.TP
/fB/-c/fR [default: config.toml]
Set configuration file path