Stop doing automatic author, about and version

[TeXitoi]

Since the begining, structopt use the CARGO_PKG environment variables to populate version, about and authors if not provided. As sometime you don't want that to be setted, there is an ugly hack that, if you do author = "", the magic is removed.

That's not so in par with the clap philosophy, as these setters are not called by default on raw clap.

Thus I propose to remove the ugly hack, don't use CARGO_PKG by default, but add without argument options to set these fiels using the CARGO_PKG, i.e.

#[derive(StructOpt)]
struct Opt {}

became

#[derive(StructOpt)]
#[structopt(about, author, version)]
struct Opt {}

and

#[derive(StructOpt)]
#[structopt(about = "", author = "", version = "")]
struct Opt {}

became

#[derive(StructOpt)]
struct Opt {}

To anyone reading this, what do you think?

Would fix #172 and maybe #173

cc @sphynx @killercup @kbknapp @ssokolow @robinst @0ndorio

[ssokolow]

I think the goal should be to default to a reasonable approximation of platform convention, similar to how Qt will try to default to matching the look of the target platform's GUI.

By that logic:

1. The current behaviour of version is perfectly reasonable as a default
2. The current behaviour of about is a footgun, because it competes with rustdoc for what purpose the struct's docstring should serve. (I just noticed another one of my projects slipped into having --help say Command-line argument schema because it decided to draw from the docstring that I include for the benefit of rustdoc.)
3. The current behaviour of author is at odds with GNU conventions, it's unusual enough that end-users are going to assume it's an opt-in decision and may mistake it for a sign that the structopt user is an egotist.

In short, I'd keep version's current behaviour but, otherwise, I'm for this.

[0ndorio]

I like the general idea to move into saner defaults and I think the changes you proposed are a step into the right direction. So just ping me as soon as you decided what to do and I will help to implement it.

But I also think we will run into some odds based on the default behavior of clap and I think it's worth to push a bit more on their side to get rid of them instead of working on structopt internal fixes. This mostly includes the following topics:

• Meta Information: Next to the version number and the author the default template output also contains the binary name on the first line. This might look okay as long as these information are combined but I think it will start to annoy as soon as we hide the version number and the author by default.

  Personally I'm expecting to see all three of them only if I use --version as none of them is actually helpful when I try to find usage information but If the binary name stays at it currents position it would be great if clap would at least introduce an empty line between the leading meta information and the about text.

• About Behavior: In contrast to @ssokolow I like to use my doc strings as an alternative to custom about and long_about arguments but I can understand that not everybody feels like this.

  I still think the main issue here is that clap always outputs App::long_about regardless if the user used -h or --help as initial argument. This becomes even worse if combined with custom templates as these forces the developer to either use {about} or {long_about} -- which by the way doesn't seem to be documented as an available placeholder but is still validated properly -- but doesn't provide a proper auto toggle version which respects the difference of -h and --help.

  Which automatically leads to the strange and hard to predict combination of arguments necessary to disable that feature we discussed in about/long_about and doc-comment precedence is unpredictable #173. Another potential solution I can see would be a structopt top level argument which disables the doc string parsing in general.

[ssokolow]

In contrast to @ssokolow I like to use my doc strings as an alternative to custom about and long_about arguments but I can understand that not everybody feels like this.

Which supports my point that "it competes with rustdoc for what purpose the struct's docstring should serve." (I see it as a situation comparable to not implementing IntoIterator for &str and String because the computer can't know whether you want bytes, codepoints, codepoint indexes, lines, etc.)

[CreepySkeleton]

I'd say:

• version behaves just as it should, I have yet to see a case when it's not welcome. I vote for keeping it as is.
• about (doc comments): actually, I always use doc comments on StructOpt structs exactly for being processed by structopt, but I can see other's people point. I abstain.
• author: honestly, I had some problems with that, I vote for changing it.

[CreepySkeleton]

@TeXitoi Do you need any help on this? I'm going to have a vacation for a week, I'm quite familiar with the codebase and I'd love to make use of the upcoming features so just tell me what are you going to do and we may work it out.

[TeXitoi]

@CreepySkeleton That would be great, I have difficulties to find enough time to do that.

I propose:

• no default author and about
• an attribute author without args that takes the cargo env
• an attribute about without args that takes the cargo env
• an attribute no_version without args that remove the version
• no special casing of "" anywhere

Also, at the same time, can you populate the CHANGELOG.md with your changes, I forgot to ask for that in your PR.

Thanks for your contribution.

[I60R]

Personally I like the current behavior as I think that it provides the best defaults for the majority of use cases and looks more pleasant. I'm also fine with not being in par with clap philosophy here.

Although, I understand why it's going to change, and I'll be also fine with that.

As an alternative might I suggest to preserve the current behavior but also enhance structopt to generate Opt::from_args_custom method for situations where current behavior isn't appropriate?

E.g.

#[derive(StructOpt)]
#[structopt(version = "1.0")]  // I want only version to be displayed in --help
struct Opt {
    ...
}

fn get_options() -> Opt {
    Opt::from_args_custom()
}

[CreepySkeleton]

I think that it provides the best defaults for the majority of use cases and looks more pleasant

It does, but it's not just about "majority of use cases", there are also "default behavior doesn't break anything" considerations. My case as an example:

I travel quite a lot with my job and sometimes I have no access to my own laptop. (Un)Fortunately, I tend to stay close to my friends so I often ask them to borrow theirs. Most of them have their own cargo installations with their own credentials, and I must admit that literally each one of my projects that needs command-line args parsing uses structopt. I'm sure you can see where we're going here - it wasn't just once when I accidentally started a project using wrong author credentials, thanks god they were closed-sourced and I had the authority to rewrite the history. (I also tend to forget to check git credentials but that's another story).

Also see @ssokolow comment about leaking internal comments into the public usage message.

So here's the point: now 9/10 of structopt users are happy with the default behavior and 1/10 users get f..reaked up if they aren't careful enough. With this changes we would have: 9/10 users will have to use #[structopt(author, about)] which is very easy and I doubt people would complain; 1/10 users will have no author/about behavior by default which is in my opinion much better than wrong behavior.

As an alternative might I suggest to preserve the current behavior but also enhance structopt to generate Opt::from_args_custom method for situations where current behavior isn't appropriate?

Sorry, I don't get it. Could you please explain how Opt::from_args_custom should work, exactly?

[I60R]

So here's the point: now 9/10 of structopt users are happy with the default behavior and 1/10 users get f..reaked up if they aren't careful enough.

But aren't they f..reaked up anyway by having the wrong credentials in Cargo.toml?

Sorry, I don't get it. Could you please explain how Opt::from_args_custom should work, exactly?

Currently we have from_args function in StructOpt trait

structopt/src/lib.rs

Lines 582 to 589 in 5dfa606

	/// Gets the struct from the command line arguments. Print the
	/// error message and quit the program in case of failure.
	fn from_args() -> Self
	where
	Self: Sized,
	{
	Self::from_clap(&Self::clap().get_matches())
	}

I propose to rewrite it as following

 /// Gets the struct from the command line arguments.  Print the 
 /// error message and quit the program in case of failure. 
 fn from_args() -> Self 
 where 
     Self: Sized, 
 { 
     Self::from_clap(&Self::clap()
        .version(env!("CARGO_PKG_VERSION"))
        .author(env!("CARGO_PKG_AUTHORS"))
        .about(env!("CARGO_PKG_DESCRIPTION"))
        .get_matches()
     ) 
 } 

And provide another function alongside which looks as from_args before

 /// Gets the struct from the command line arguments.  Print the 
 /// error message and quit the program in case of failure. Default
 /// entries from Cargo.toml will not be populated
 fn from_args_custom() -> Self 
 where 
     Self: Sized, 
 { 
     Self::from_clap(&Self::clap().get_matches()) 
 } 

It will be fully backward compatible way. However, it might have drawback that from_iter and from_iter_safe functions from StructOpt trait as well should have _custom duplicate, and users might be confused version, about, author on Opt doesn't have any effect. So, this might be wrong solution

[TeXitoi]

I don't want a from_arg_custom, that overly complicated.

That's an API design choice, there is no good or bad somution, only traid of. That would be only me, I'd do my proposition for clap consistency. But clap is not really thought with no version in mind, and that's why I propose to keep version.

Also, note that the about thing is only about the description from the toml. The doc comment thing is some other thing, not related to this issue.

And for the author thing, I prefer the no author by default, more consistent with others and clap's defaults.

[CreepySkeleton]

@TeXitoi I found some time and I'm working on it now, should be done by tomorrow. Just to be sure I get it right, feel free to correct:

• version/author/about = "whatever" methods stay intact
• the version/about/author = "" special cases are to be removed and should trigger an error
• add about/author methods with no args - they take their value from corresponding environment variables
• also we introduce no_version no args method which prevents "the version by default is to be taken from Cargo.toml` behavior.
• version, unlike author/about, keeps its old behavior unless no_version mentioned

[TeXitoi]

Exact, except:

the version/about/author = "" special cases are to be removed and should trigger an error

Error is not needed, just .about("") called is OK. If you prefer to do an error (maybe to explicitly warn the user of the old behavior), I'm OK if it doesn't complexify the code.

[CreepySkeleton]

maybe to explicitly warn the user of the old behavior

Exactly my point, people tend to stick to patterns they got used to, in my experience such explicit errors save lots of hours and sanity. The only thing - issuing a warning is impossible on stable rust so it's going to have to be a hard error.

if it doesn't complexify the code

It shouldn't - an extra match or if statement will suffice