Re: new function passphrase generator

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Tue, Sep 2, 2025 at 3:50 AM Clinton Bunch <cdb_zsh@xxxxxxxxxxx> wrote:
>
> Uses zsh/random and diceware to generate a passphrase.  I seem to have
> problems with my completion setup so I wasn't able to do serious testing
> on the completion function.  Hopefully someone will test it out.

Hi,

A couple of thoughts,

"Here's a completer but idk if it works" isn't the best approach to
submitting things.

A quick web search suggests that there are plenty of "diceware"
generators on github and other sites, and many of them are also named
"diceware", so naming the zsh function (which is not loaded by
default) and the completer (loaded by default) also diceware is
probably not going to be the best end user experience for people using
those other tools. This could easily be named zdiceware or something
instead.

Zsh doesn't use xdg config directories, so searching in those
directories is a little questionable, but I'm not objecting to it I
suppose. Not being able to customize the path at all seems like a
bigger problem. There is supposedly a "default" path and a "default"
wordlist filename, but you cannot set these independently, only a full
path to a wordlist, which feels inconsistent and a bit weird.

Also, the code has comments like this: "Correct Zsh specific syntax
for sorting numerically". What is the point of such a comment, should
we assume that lines of code without this comment are using incorrect
syntax? :)

"word="${(C)word}" # Capitalize the first letter using Zsh (C) flag"
comments like this are overly verbose of the type "initialize i to 0"
and should not be included in my opinion. Meanwhile, this gobbledygook
is completely uncommented,
> +for opt_name opt_val in "${(@kv)parsed_opts}"; do
> +  if [[ $opt_val =~ '^=.*' && $#opt_val -gt 1 ]]; then
> +    opt_val="${opt_val#=}"

> +# Requires: zsh/random module, sed
> +
> +# Load the zsh/random module early. Critical failure if not available.
> +if ! zmodload zsh/random; then
> +  print -u2 "Error: zsh/random module could not be loaded. Is it
> installed and available?"
> +  return 1
> +fi

But if sed is missing, it's fine to generate an empty password later on?

> +# Declare option variables with default values and `integer` type
> +integer capitalize=0
> +local separator=" "
> +integer num_to_add_to_words=0
> +integer num_words=6 # Default number of words
More overly obvious comments


> +# Enforce minimum number of words
> +if (( num_words < 2 )); then
> +  print -u2 "Error: Minimum number of words is 2."
Mhm.

> +# Determine wordlist file if not explicitly specified
> +if [[ -z "$wordlist_file" ]]; then
> +  for p in "${(@e)wordlist_search_paths}"; do
> +    if [[ -f "$p/$default_wordlist_file" ]]; then
> +      wordlist_file="$p/$default_wordlist_file"
> +      break
> +    fi
> +  done
> +
> +  if [[ -z "$wordlist_file" ]]; then
> +    print -u2  "
> +Error: Diceware wordlist 'eff_large_wordlist.txt' not found in any
> search path.

This error message has a filename hardcoded.

> +Please download the official wordlist from the Electronic Frontier
> Foundation
> +and place it in one of the searched directories (e.g., ~/.config/zsh/).

This one helpfully only lists a non-supported config directory for
zsh, other than by this script.

> +integer num_lines=$(wc -l < "$wordlist_file" | tr -d ' ')
I was told only zsh/random and sed were required, but here are wc and
tr making a guest appearance.


> +local sed_command=""
> +if (( num_words > 0 )); then
> +    # Correct Zsh specific syntax for sorting numerically, appending
> 'p', and joining
> +    local -a tmp_sed_parts=(${^${(n)line_numbers}}p)
> +    sed_command="${(j:;:)tmp_sed_parts}"
> +fi
> +
> +local -a raw_words
> +if [[ -n "$sed_command" ]]; then
> +    raw_words=( $(sed -n "$sed_command" "$wordlist_file") )
> +else
> +    raw_words=() # No words to fetch if num_words was 0
> +fi

What happens if this sed command fails? Nothing good, it seems like.
(And this should still be checked for failure even if existence of sed
is tested for in the other place above.)

> +local -a passphrase_words=()
> +local x word
> +for x word in "${raw_words[@]}"; do
> +  if (( capitalize )); then
> +    word="${(C)word}" # Capitalize the first letter using Zsh (C) flag
> +  fi
> +  passphrase_words+=( "$word" )
> +done

These words can be capitalized outside the loop, instead of inside
(Not that the performance difference matters).

> +local final_passphrase="${(pj:$separator:)passphrase_words}"
> +
> +print "$final_passphrase"

Might want to use print -r - here, in case someone uses a \ as the
separator or something?

-- 
Mikael Magnusson