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 uutils/coreutils mktemp command into nushell #10845

Closed
tskinn opened this issue Oct 26, 2023 · 4 comments · Fixed by #11005
Closed

add uutils/coreutils mktemp command into nushell #10845

tskinn opened this issue Oct 26, 2023 · 4 comments · Fixed by #11005
Labels
coreutils-uutils Changes relating to coreutils/uutils enhancement New feature or request
Milestone

Comments

@tskinn
Copy link
Contributor

tskinn commented Oct 26, 2023

Related problem

mktemp on macos and linux behave differently. It would be awesome to have a single cross platform implementation in nu.

Describe the solution you'd like

Create new nushell command mktemp using uutils/coreutils.

@tskinn tskinn added enhancement New feature or request needs-triage An issue that hasn't had any proper look labels Oct 26, 2023
@fdncred fdncred added coreutils-uutils Changes relating to coreutils/uutils and removed needs-triage An issue that hasn't had any proper look labels Oct 26, 2023
@fdncred
Copy link
Collaborator

fdncred commented Oct 26, 2023

Sounds good to me.

@tskinn
Copy link
Contributor Author

tskinn commented Oct 26, 2023

Are there any barriers to implementing this? Would this be considered a breaking change? Does it matter at this point or should we make a command with a different name like temp dir / temp file or something like that.

@fdncred
Copy link
Collaborator

fdncred commented Oct 26, 2023

I'd prefer to use the uu_mktemp crate. I'm not sure what parameters it has or if the coreutils team has exposed the proper functionality so we can use it. You may just have to give it a try.

@tskinn
Copy link
Contributor Author

tskinn commented Oct 28, 2023

Linking to uutils/coreutils#5465

fdncred added a commit that referenced this issue Nov 18, 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](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>
@hustcer hustcer added this to the v0.88.0 milestone Nov 18, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this issue 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 issue 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 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants