[pve-devel] [pve-common PATCH] Refactoring read_password.

[Dietmar Maurer]

comments inline:

> On November 3, 2016 at 9:06 AM Wolfgang Link <w.link at proxmox.com> wrote:
> 
> 
> We use this function in 3 different packets with the same code.
> 
> It will moved to the CLIHandler, because we need it only on the command line.
> ---
>  src/PVE/CLIHandler.pm | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index e2a81a0..580040e 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -8,6 +8,7 @@ use PVE::SafeSyslog;
>  use PVE::Exception qw(raise raise_param_exc);
>  use PVE::RESTHandler;
>  use PVE::INotify;
> +use Term::ReadLine;
>  
>  use base qw(PVE::RESTHandler);
>  
> @@ -48,6 +49,30 @@ my $complete_command_names = sub {
>      return $res;
>  };
>  
> +sub read_password_impl{
> +    my ($command, $set_pass, $undo_shadow) = @_;

I am a bit confused by the parameters. Please can you explain
why we need $set_pass and $undo_shadow ?

> +    my $term = new Term::ReadLine($command);
> +    my $attribs = $term->Attribs;
> +    my $redisplay = $attribs->{redisplay_function};
> +    $attribs->{redisplay_function} = $attribs->{shadow_redisplay};
> +
> +    my $remove_hist = $term->Features->{autohistory} ? 1 : 0;
> +
> +    my $input = undef;
> +    $input = $term->readline('Enter password: ');
> +    $term->remove_history($term->where_history()) if $remove_hist;
> +
> +    my $conf = $term->readline('Retype password: ') if $set_pass;

s/$conf/$confirm/ 

And maybe 'verify' would be a better name for the 'set_pass' parameter?

> +    $term->remove_history($term->where_history()) if $remove_hist;
> +
> +    $attribs->{redisplay_function} = $redisplay if $undo_shadow;

What happens exactly if $undo_shadow is false?