Fix incorrect help message

[Phlosioneer]

The cargo clean --target command does not have a default setting, but the command-line help text says it does.

[rust-highfive]

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

[ehuss]

I think what it is trying to say is that if you don't specify --target, it cleans all of them. i.e., that's the default behavior without the flag.

[dwijnand]

What @ehuss said ^

Perhaps:

Suggested change

	.arg_target_triple("Target triple to clean output for")
	.arg_target_triple("Target triple to clean output for (defaults to all targets)")

[Phlosioneer]

It's confusing to me because it's the only one that specifies the default behavior. It led me to believe that cargo clean --target, without a triple, would default to "all".

I found what I was looking for when I discovered that cargo clean --target all is accepted.

I still think it should be removed as the only default behavior explanation, but you can close this PR if you want.

[dwijnand]

Looks like there is a .default_value:

cargo/src/cargo/util/command_prelude.rs

Lines 133 to 141 in 26232c3

	fn arg_message_format(self) -> Self {
	self._arg(
	opt("message-format", "Error format")
	.value_name("FMT")
	.case_insensitive(true)
	.possible_values(&["human", "json", "short"])
	.default_value("human"),
	)
	}

Maybe in this PR we should move the mention of "all" from the description to a call to .default_value in arg_target_triple:

cargo/src/cargo/util/command_prelude.rs

Lines 119 to 121 in 26232c3

	fn arg_target_triple(self, target: &'static str) -> Self {
	self._arg(opt("target", target).value_name("TRIPLE"))
	}

[ehuss]

After thinking about it some more, perhaps it should be removed? cargo clean is not the most precise of commands, and is a little fuzzy in how it works. If you do cargo clean -p pkgname, then indeed it only cleans using the normal convention (no --target cleans host only). I think some of the confusion is that bare cargo clean just deletes everything, and is a bit special.

So I think I am 👍 on this now.

[ehuss]

Hm, but there is a bit of a bug where cargo clean --target xxx ignores the --target flag. That seems like a separate issue. Although given #3530 (comment) maybe that's confusing? Hmm...

[Phlosioneer]

I think I like @dwijnand’s idea to clarify that the default is for when --target is omitted, not when the target triple is omitted.

Would “(normally cleans all targets)” work? It avoids the word default so that there is no confusion over whether the target triple argument has a default.

[Phlosioneer]

@ehuss I might look into that separate bug when I get home. If I can figure it out, I’ll make a PR for it. Shouldn’t be too hard.

[ehuss]

Would “(normally cleans all targets)” work? It avoids the word default so that there is no confusion over whether the target triple argument has a default.

Except it doesn't always clean all targets. The only time it does that is if you don't provide a package (i.e. a bare cargo clean). I would just remove the parenthetical. And maybe update the after-help to make it a little clearer that cargo clean without any options just deletes the entire target directory. That's maybe implied by "all packages' artifacts are removed", but that could maybe be a little clearer.

[ehuss]

@dwijnand Do you object to merging this as-is, and then following up later with more documentation about how cargo clean works? It might also be an opportunity to survey the deficiencies of cargo clean and sketch out some fixes to make it work better.

[dwijnand]

Sure, I'm always down for incremental improvements and eagerly landing changes.

@bors r+

[bors]

📌 Commit 26232c3 has been approved by dwijnand

[bors]

⌛ Testing commit 26232c3 with merge 6022bbb...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: dwijnand
Pushing 6022bbb to master...