Great, thanks Kevin. My reasons for including the environment variable configuration probably had to do with the fact that I chose 15 characters as the default pretty arbitrarily. It seems long enough to me, but maybe some folks with more security needs might want it to default to 20, for example. I wasn't sure exactly what the best default length would be, so that's partly why I included the override. But I think you make a good point about just overriding it on the command line, so I'll defer to you all!
Nathan Wallace nosuchthingastwo.com On Fri, Dec 18, 2015 at 7:13 AM, Kevin Cox <[email protected]> wrote: > I think this is a great idea and the code looks good at a quick glance. A > small bikeshed comment is that I don't think that default is required in > the environment variable because it is probably expected that the command > line argument will override. So 'PASSWORD_STORE_LENGTH' would be sufficient > IMHO. > On Dec 18, 2015 12:18 AM, "Nathan Wallace" <[email protected]> > wrote: > >> --- >> src/password-store.sh | 9 ++++++--- >> tests/t0010-generate-tests.sh | 6 ++++++ >> 2 files changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/src/password-store.sh b/src/password-store.sh >> index d535a74..ad6d367 100755 >> --- a/src/password-store.sh >> +++ b/src/password-store.sh >> @@ -15,6 +15,7 @@ which gpg2 &>/dev/null && GPG="gpg2" >> PREFIX="${PASSWORD_STORE_DIR:-$HOME/.password-store}" >> X_SELECTION="${PASSWORD_STORE_X_SELECTION:-clipboard}" >> CLIP_TIME="${PASSWORD_STORE_CLIP_TIME:-45}" >> +DEFAULT_LENGTH="${PASSWORD_STORE_DEFAULT_LENGTH:-15}" >> >> export GIT_DIR="${PASSWORD_STORE_GIT:-$PREFIX}/.git" >> export GIT_WORK_TREE="${PASSWORD_STORE_GIT:-$PREFIX}" >> @@ -234,8 +235,9 @@ cmd_usage() { >> overwriting existing password unless forced. >> $PROGRAM edit pass-name >> Insert a new password or edit an existing password using >> ${EDITOR:-vi}. >> - $PROGRAM generate [--no-symbols,-n] [--clip,-c] >> [--in-place,-i | --force,-f] pass-name pass-length >> + $PROGRAM generate [--no-symbols,-n] [--clip,-c] >> [--in-place,-i | --force,-f] pass-name [pass-length] >> Generate a new password of pass-length with optionally no >> symbols. >> + If pass-length is not specified, the password will be >> $DEFAULT_LENGTH characters long. >> Optionally put it on the clipboard and clear board after >> $CLIP_TIME seconds. >> Prompt before overwriting existing password unless forced. >> Optionally replace only the first line of an existing >> file with a new password. >> @@ -441,9 +443,10 @@ cmd_generate() { >> --) shift; break ;; >> esac done >> >> - [[ $err -ne 0 || $# -ne 2 || ( $force -eq 1 && $inplace -eq 1 ) >> ]] && die "Usage: $PROGRAM $COMMAND [--no-symbols,-n] [--clip,-c] >> [--in-place,-i | --force,-f] pass-name pass-length" >> + [[ $err -ne 0 || ( $# -ne 1 && $# -ne 2 ) || ( $force -eq 1 && >> $inplace -eq 1 ) ]] && >> + die "Usage: $PROGRAM $COMMAND [--no-symbols,-n] >> [--clip,-c] [--in-place,-i | --force,-f] pass-name [pass-length]" >> local path="$1" >> - local length="$2" >> + local length="${2:-$DEFAULT_LENGTH}" >> check_sneaky_paths "$path" >> [[ ! $length =~ ^[0-9]+$ ]] && die "Error: pass-length >> \"$length\" must be a number." >> mkdir -p -v "$PREFIX/$(dirname "$path")" >> diff --git a/tests/t0010-generate-tests.sh b/tests/t0010-generate-tests.sh >> index cadb76f..5212f8d 100755 >> --- a/tests/t0010-generate-tests.sh >> +++ b/tests/t0010-generate-tests.sh >> @@ -16,4 +16,10 @@ test_expect_success 'Test replacement of first line' ' >> [[ $("$PASS" show cred2) == "$(printf "This is a fake >> password\\npassword\\nwith\\nmany\\nlines\\nin it bla bla")" ]] >> ' >> >> +test_expect_success 'Test "generate" default length' ' >> + "$PASS" init $KEY1 && >> + "$PASS" generate cred3 && >> + [[ $("$PASS" show cred3 | wc -m) -eq 16 ]] >> +' >> + >> test_done >> -- >> 2.5.0 >> >> _______________________________________________ >> Password-Store mailing list >> [email protected] >> http://lists.zx2c4.com/mailman/listinfo/password-store >> >
_______________________________________________ Password-Store mailing list [email protected] http://lists.zx2c4.com/mailman/listinfo/password-store
