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

Add mktemp command #11005

Merged
merged 4 commits into from Nov 18, 2023
Merged

Add mktemp command #11005

merged 4 commits into from Nov 18, 2023

Conversation

tskinn
Copy link
Contributor

@tskinn tskinn commented Nov 9, 2023

closes #10845

I've opened this a little prematurely to get some questions answered before I cleanup the code.

As I started trying to better understand GNUs mktemp I've realized its kind of peculiar and we might want to change its behavior to introduce it to nushell.

quiet and dry run

Does it make sense to keep the quiet and dry_run flags? I don't think so. The GNU documentation says this about the dry run flag "Using the output of this command to create a new file is inherently unsafe, as there is a window of time between generating the name and using it where another process can create an object by the same name." So yeah why keep it? As far as quiet goes, does it make sense to silence the errors in nushell?

other confusing flags

According to the gnu docs, the -t flag is deprecated and the -p/ --tempdir are the same flag with the only difference being --tempdir takes an optional path, Given that, I've broken the -p away from --tempdir. Now there is one switch --tmpdir/-t and one named param --tmpdir-path/-p.

GNU mktemp

  -p DIR, --tmpdir[=DIR]  interpret TEMPLATE relative to DIR; if DIR is not
                        specified, use $TMPDIR if set, else /tmp.  With
                        this option, TEMPLATE must not be an absolute name;
                        unlike with -t, TEMPLATE may contain slashes, but
                        mktemp creates only the final component
  -t                  interpret TEMPLATE as a single file name component,
                        relative to a directory: $TMPDIR, if set; else the
                        directory specified via -p; else /tmp [deprecated]

to
nushell mktemp

  -p, --tmpdir-path <Filepath> # named param, must provide a path
  -t, --tmpdir                 # a switch

Is this a terrible idea?

What should I do?

@fdncred
Copy link
Collaborator

fdncred commented Nov 9, 2023

We're fine with starting with a minimal set of flags as a first pass and then adding more later if people need them.

I'm fine with your -p/-t suggestion. If we need to change it later for some reason, we can. I wouldn't want there to be any huge surprises to people who use uutils/coreutils mktemp though.

@fdncred fdncred added the coreutils-uutils Changes relating to coreutils/uutils label Nov 9, 2023
@fdncred
Copy link
Collaborator

fdncred commented Nov 16, 2023

I see your uu_mktemp PR landed. It would be great to get this in soon. Hope you have time to work on it!

@tskinn tskinn marked this pull request as ready for review November 17, 2023 03:43
@tskinn
Copy link
Contributor Author

tskinn commented Nov 17, 2023

K I think it is about ready. Do I need to worry about the Cargo.lock file?

@fdncred
Copy link
Collaborator

fdncred commented Nov 17, 2023

K I think it is about ready. Do I need to worry about the Cargo.lock file?

Yes, you should include the Cargo.lock file since I assume it has changed.

@fdncred
Copy link
Collaborator

fdncred commented Nov 18, 2023

Thanks for working on this!

@fdncred fdncred merged commit 494a5a5 into nushell:main Nov 18, 2023
20 checks passed
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
closes nushell#10845 

I've opened this a little prematurely to get some questions answered
before I cleanup the code.

As I started trying to better understand GNUs `mktemp` I've realized its
kind of peculiar and we might want to change its behavior to introduce
it to nushell.

#### quiet and dry run

Does it make sense to keep the `quiet` and `dry_run` flags? I don't
think so. The GNU documentation says this about the dry run flag "Using
the output of this command to create a new file is inherently unsafe, as
there is a window of time between generating the name and using it where
another process can create an object by the same name." So yeah why keep
it? As far as quiet goes, does it make sense to silence the errors in
nushell?

#### other confusing flags

According to the [gnu
docs](https://www.gnu.org/software/coreutils/manual/html_node/mktemp-invocation.html),
the `-t` flag is deprecated and the `-p`/ `--tempdir` are the same flag
with the only difference being `--tempdir` takes an optional path, Given
that, I've broken the `-p` away from `--tempdir`. Now there is one
switch `--tmpdir`/`-t` and one named param `--tmpdir-path`/`-p`.

GNU mktemp
```
  -p DIR, --tmpdir[=DIR]  interpret TEMPLATE relative to DIR; if DIR is not
                        specified, use $TMPDIR if set, else /tmp.  With
                        this option, TEMPLATE must not be an absolute name;
                        unlike with -t, TEMPLATE may contain slashes, but
                        mktemp creates only the final component
  -t                  interpret TEMPLATE as a single file name component,
                        relative to a directory: $TMPDIR, if set; else the
                        directory specified via -p; else /tmp [deprecated]

```
to
nushell mktemp
```
  -p, --tmpdir-path <Filepath> # named param, must provide a path
  -t, --tmpdir                 # a switch
```

Is this a terrible idea?

What should I do?

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
closes nushell#10845 

I've opened this a little prematurely to get some questions answered
before I cleanup the code.

As I started trying to better understand GNUs `mktemp` I've realized its
kind of peculiar and we might want to change its behavior to introduce
it to nushell.

#### quiet and dry run

Does it make sense to keep the `quiet` and `dry_run` flags? I don't
think so. The GNU documentation says this about the dry run flag "Using
the output of this command to create a new file is inherently unsafe, as
there is a window of time between generating the name and using it where
another process can create an object by the same name." So yeah why keep
it? As far as quiet goes, does it make sense to silence the errors in
nushell?

#### other confusing flags

According to the [gnu
docs](https://www.gnu.org/software/coreutils/manual/html_node/mktemp-invocation.html),
the `-t` flag is deprecated and the `-p`/ `--tempdir` are the same flag
with the only difference being `--tempdir` takes an optional path, Given
that, I've broken the `-p` away from `--tempdir`. Now there is one
switch `--tmpdir`/`-t` and one named param `--tmpdir-path`/`-p`.

GNU mktemp
```
  -p DIR, --tmpdir[=DIR]  interpret TEMPLATE relative to DIR; if DIR is not
                        specified, use $TMPDIR if set, else /tmp.  With
                        this option, TEMPLATE must not be an absolute name;
                        unlike with -t, TEMPLATE may contain slashes, but
                        mktemp creates only the final component
  -t                  interpret TEMPLATE as a single file name component,
                        relative to a directory: $TMPDIR, if set; else the
                        directory specified via -p; else /tmp [deprecated]

```
to
nushell mktemp
```
  -p, --tmpdir-path <Filepath> # named param, must provide a path
  -t, --tmpdir                 # a switch
```

Is this a terrible idea?

What should I do?

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coreutils-uutils Changes relating to coreutils/uutils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add uutils/coreutils mktemp command into nushell
2 participants