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

Password option behaviour is unintuitive and inflexible #70

Closed
rodionmoiseev opened this issue Jun 18, 2011 · 3 comments
Closed

Password option behaviour is unintuitive and inflexible #70

rodionmoiseev opened this issue Jun 18, 2011 · 3 comments

Comments

@rodionmoiseev
Copy link

The following I find unintuitive and inflexible:

  1. The current password interface does not seem to provide any way to specify the password as an argument. (e.g. -pass=secret). A way to set the password in command line or to read it from some @passwd.txt would be nice.
  2. If password has required=true set, then the user is forced to pass the --pass argument just to get a password prompt on the screen. If the password is required there should be no need to specify --pass on the command line.

In fact, number 2. applies to all boolean options with required=true set. It does not really make sense to have required boolean options, unless the user can do something like --debug=false

Back to the point, I'd like to propose an interface like this:

  • Add a new annotation for passwords, let's say @PasswordParameter.
    • @PasswordParameter(names = {"-p", "--pass"}) behaves like a standard String option (i.e. has arity of 1).
    • Additionally, @PasswordParameter(flags = {"--prompt"}) behaves like the current implementation (i.e. has arity of 0).
  • If password field is optional (default), then passing -p=secret would get the password from cmd, and --prompt would ask for password in the console. If no options specified not password is set and the program proceeds as usual.
  • If password field is required, then specifying any options on the cmd behaves the same as when it's optional. If no options are specified, no error is generated and password is retrieved from the prompt.

The @PasswordParameter annotation will have all the same fields as in standard @Parameter, except password and with an additional String[] flags() default {}.

@cbeust
Copy link
Owner

cbeust commented Jun 18, 2011

Not sure I follow:

  1. The password attribute is here only to provide your password on stdin with hidden input. If you want to pass the password without any interactive input, just make it a regular string parameter.

  2. Possibly, although forcing the argument allows to specify the string to display, which can then be customized ("Enter the password to your database").

Overall, your proposal is reasonable and I should probably have done that in the first place, but it overlaps with what JCommander is currently doing so I think it might introduce some confusion, unless I completely remove the password attribute to @parameter, which is not really feasible since it would break backward compatibility.

@rodionmoiseev
Copy link
Author

  1. Whether to specify the password as an argument, from a file, or via console should be up to the user (I mean, user that uses my software that I make using JCommander. That user is not me). Therefore, ideally I want to have all input methods available with a single password parameter.
  2. I don't see the problem. The customized message is the one specified by description = "Enter the password to your db", right? In that case jcommander should be able to display the same message regardless of whether --pass was given in the arguments or not.

I thought about compatibility. It's true that you'll have to keep backwards compatibility in terms of API usage and behaviour. For this reason, my proposal simply adds on top, and therefore users of the old API will have the same behaviour. I understand that having two APIs for doing the same thing is not very elegant, but there is always an option to mark the old API deprecated. Let me know what you think.

@jeremysolarz
Copy link
Contributor

@cbeust no action for a long time. I guess with fixing the password prompt as requested in #195 we have covered a lot of use cases.

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

No branches or pull requests

3 participants