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

cp: --reflink with no argument should be assumed --reflink=always #3984

Closed
jfinkels opened this issue Sep 27, 2022 · 4 comments · Fixed by #3992
Closed

cp: --reflink with no argument should be assumed --reflink=always #3984

jfinkels opened this issue Sep 27, 2022 · 4 comments · Fixed by #3992
Labels

Comments

@jfinkels
Copy link
Collaborator

The --reflink command line option to cp takes an optional argument (always, auto, or never, with always being the default) but it can be used without an argument. uutils cp does not allow this type of usage, but should

GNU cp v8.30:

$ touch f && cp --reflink f g
cp: failed to clone 'g' from 'f': Operation not supported

uutils cp:

$ touch f && ./target/debug/cp --reflink f g
./target/debug/cp: invalid argument 'f' for 'reflink'

This is similar to an issue with mktemp --tmpdir, described here:

// Special case to work around a limitation of `clap`; see
// `is_tmpdir_argument_actually_the_template()` for more
// information.
//
// Fixed in clap 3
// See https://github.com/clap-rs/clap/pull/1587
let (tmpdir, template) = if is_tmpdir_argument_actually_the_template(matches) {

@sssemil
Copy link
Contributor

sssemil commented Sep 29, 2022

Can confirm. Working on this.

@sssemil
Copy link
Contributor

sssemil commented Sep 29, 2022

Hm, so I can add .default_missing_value("auto") but then it still tries to take the file name as an argument. But I can also then set .require_equals(true), however it will fail if nothing is specified because = is required. It looks like there's no softer version of require_equals in clap.

@tertsdiepraam
Copy link
Member

Maybe this can be of help? This is how we did the --color argument for ls.

coreutils/src/uu/ls/src/ls.rs

Lines 1428 to 1436 in 043c009

Arg::new(options::COLOR)
.long(options::COLOR)
.help("Color output based on file type.")
.takes_value(true)
.value_parser([
"always", "yes", "force", "auto", "tty", "if-tty", "never", "no", "none",
])
.require_equals(true)
.min_values(0),

@sssemil
Copy link
Contributor

sssemil commented Sep 29, 2022

@tertsdiepraam that works, thanks! Will make a PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants