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

feat: shorthands for --allow-* flags #19530

Closed
KnorpelSenf opened this issue Jun 16, 2023 · 11 comments
Closed

feat: shorthands for --allow-* flags #19530

KnorpelSenf opened this issue Jun 16, 2023 · 11 comments
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@KnorpelSenf
Copy link
Contributor

KnorpelSenf commented Jun 16, 2023

It is tedious to type --allow-env --allow-net --allow-read and so on. Ry even said in #668 (review) that he would not be opposed to single-character permission flags.

I suggest to add the follow flags to the CLI:

# short long            comment

  -A    --allow-all     # exists already
  -E    --allow-env
  -F    --allow-ffi
  -T    --allow-hrtime
  -N    --allow-net
  -R    --allow-read    # could this be confused with `-r` for `--reload`?
  -X    --allow-run
  -S    --allow-sys
  -W    --allow-write

In scripts, it is usually preferable to have longer, more explicit names. They are better at being transparent about which permissions are granted to the program.

However, when running a lot of scripts as one-off commands from the shell, it is very cumbersome to type out the current arguments. It would be very nice to be able to do just deno run -ERN and be done with it. (Currently, it is necessary to type deno run --allow-env --allow-read --allow-net.)

What do you think about this?

@benatkin
Copy link

benatkin commented Jun 16, 2023

This is not the case:

Ry even said in #668 (review) that he would like to see single-character permission flags.

In the linked issue #668 he says:

I'm not opposed to having short hand flags for the permission flags

Not the same thing as wanting to see them.

It's also almost 5 years old.

@KnorpelSenf
Copy link
Contributor Author

KnorpelSenf commented Jun 16, 2023

Fair enough, he said that he doesn't like the long form and he also doesn't want to deviate from standard args parsing, so I inferred that single-letter flags are the only other option and that we would support them instead. I corrected my wording.

Are there any reasons against this change?

@benatkin
Copy link

benatkin commented Jun 16, 2023

More misrepresentation of what he said. :(

it certainly isn't very ergonomic as it is now

That doesn't mean he doesn't like the long form.

This won't work because it takes a lot of the valuable single character namespace for a single part of it, while at the same time obscuring security features. Hopefully it will be declined. There's some upvotes, but fortunately Deno is a principled project.

@KnorpelSenf
Copy link
Contributor Author

KnorpelSenf commented Jun 16, 2023

Perhaps we should just ask him what his opinion is in 2023 rather than speculating what he could have meant five years ago. @ry?

@wojpawlik
Copy link

-RW=. would be cool.

@samyosm
Copy link

samyosm commented Jun 18, 2023

In the event this is getting accepted, I'm making a pull request.

@dsherret
Copy link
Member

dsherret commented Jul 4, 2023

I don't think this one should be implemented because it obscures the meaning of the permission flags, which degrades the permission system. Even -A for --allow-all is questionable, but what's done is done and at least there's only one short flag to remember and people are familiar with it at this point.

@dsherret dsherret added the suggestion suggestions for new features (yet to be agreed) label Jul 4, 2023
@benatkin
Copy link

benatkin commented Jul 4, 2023

@dsherret I agree. I also don't think -A is used as much of the time by Deno experts as it used to be. I'm going to withdraw my request for a single-character option for --no-prompt. #18761

@scarf005
Copy link
Contributor

scarf005 commented Jul 8, 2023

how about --allow=net,read syntax?

@KnorpelSenf
Copy link
Contributor Author

bash already gives us

$ echo --allow-{read,net}
--allow-read --allow-net

@bartlomieju
Copy link
Member

As it stands there's no support for this feature among the core team. We are more interested in exploring alternative solutions like #12763. Closing the issue for now, but feel free to leave comments if you have other opinions, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

7 participants