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.

Sign up for GitHub

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

Jump to bottom

mktemp: replace workaround code with features of clap 3 #3454

Closed
jfinkels opened this issue Apr 29, 2022 · 1 comment · Fixed by #4342
Closed

mktemp: replace workaround code with features of clap 3 #3454

jfinkels opened this issue Apr 29, 2022 · 1 comment · Fixed by #4342
Labels
good first issue For newcomers! U - mktemp

Comments

@jfinkels
Copy link
Collaborator

This code has a comment describing a workaround for a missing feature in clap that was not available until clap 3:

coreutils/src/uu/mktemp/src/mktemp.rs

Lines 89 to 98 in bf3a86f

// Special case to workaround a limitation of clap when doing
// mktemp --tmpdir apt-key-gpghome.XXX
// The behavior should be
// mktemp --tmpdir $TMPDIR apt-key-gpghome.XX
// As --tmpdir is empty
//
// Fixed in clap 3
// See https://github.com/clap-rs/clap/pull/1587
let tmp = env::temp_dir();
(tmpdir, tmp)

Perhaps some code here can be simplified now that clap 3 is available?
@sylvestre sylvestre added the good first issue For newcomers! label Apr 29, 2022
@djhworld
Copy link

djhworld commented May 4, 2022 •
edited
Loading

I'm not sure if the linked issue in clap 3 fixes the problem.

The issue stems from the fact that --tmpdir will never be empty if a template is passed (e.g. mktemp --tmpdir apt-key-gpghome.XXX)

clap will interpret the value of --tmpdir to be apt-key-gpghome.XXX rather than "" hence why this workaround is in place.

Adding default_missing_value doesn't fix it because --tmpdir is always going to be populated when a 'template' is passed.

@tmccombs tmccombs mentioned this issue Feb 11, 2023
Open
2 tasks
tmccombs added a commit to tmccombs/coreutils that referenced this issue Feb 11, 2023
@tmccombs
Require = for --tmpdir in mktemp
1caf8a9 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
@tmccombs tmccombs mentioned this issue Feb 11, 2023
Merged
tmccombs added a commit to tmccombs/coreutils that referenced this issue Feb 13, 2023
@tmccombs
Require = for --tmpdir in mktemp
c484b6e 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tmccombs added a commit to tmccombs/coreutils that referenced this issue Feb 13, 2023
@tmccombs
Require = for --tmpdir in mktemp
49ddd6f 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tmccombs added a commit to tmccombs/coreutils that referenced this issue Mar 30, 2023
@tmccombs
Require = for --tmpdir in mktemp
b45b960 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tmccombs added a commit to tmccombs/coreutils that referenced this issue Apr 3, 2023
@tmccombs
Require = for --tmpdir in mktemp
00aeb99 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tmccombs added a commit to tmccombs/coreutils that referenced this issue Jun 21, 2023
@tmccombs
Require = for --tmpdir in mktemp
5b84d2c 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tmccombs added a commit to tmccombs/coreutils that referenced this issue Jun 22, 2023
@tmccombs
Require = for --tmpdir in mktemp
904a722 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
tertsdiepraam pushed a commit to tmccombs/coreutils that referenced this issue Jul 5, 2023
@tmccombs @tertsdiepraam
Require = for --tmpdir in mktemp
5a07e58 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
sylvestre pushed a commit to tmccombs/coreutils that referenced this issue Jul 18, 2023
@tmccombs @sylvestre
Require = for --tmpdir in mktemp
b77a08c 
Fixes uutils#3454

This implementation more closely matches the behavior of GNU mktemp. Specifically,
if using the short-form `-p`, then DIR is required, and if using the long form `--tmpdir`
then DIR is optional, but if provided, must use the `--tmpdir=DIR` form (not `--tempdir DIR`),
so that there is no ambiguity with also passing a TEMPLATE.

The downside to this implementation is we are now introducing a new workaround for
clap-rs/clap#4702 where we use separate calls to `.arg()` for
the short `-p` and the long `--tmpdir`, which results in a less than ideal output for `--help`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue For newcomers! U - mktemp
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Require = for --tmpdir in mktemp tmccombs/coreutils
3 participants
@jfinkels @djhworld @sylvestre