wayland clipboard -- spam filter missed your patch

Jason A. Donenfeld Jason at zx2c4.com
Mon Feb 25 21:26:08 CET 2019


Hey Brett,

I'm just now seeing your nice wayland patch on the mailing list
archives, as it seems to have gone into my spam folder or was
otherwise not delivered into my inbox. I'd be happy to merge something
like this but with some small tweaks. Review is below.

Jason

>  .BR tr (1),
>  .BR git (1),
>  .BR xclip (1),
> -.BR qrencode (1).
> +.BR qrencode (1),
> +.BR wl-clipboard (1).

Can you place this below xclip, not below qrencode?


>  clip() {
> + # wl-clipboard's copy and paste is split into separate binaries.

Remove this comment.

> + if [[ -n $WAYLAND_DISPLAY ]] && type {wl-copy,wl-paste} &>/dev/null; then

You can assume the utilities are there, just like we do xclip.

> + local copy_cmd="wl-copy"
> + local paste_cmd="wl-paste -n"
> + if [[ $X_SELECTION == "primary" ]]; then
> + copy_cmd+=" --primary"
> + paste_cmd+=" --primary"
> + fi
> + local sleep_argv0="password store sleep on display $WAYLAND_DISPLAY"
> + else
> + local copy_cmd="${PASSWORD_STORE_copy_cmd:-xclip -selection "$X_SELECTION"}"
> + local paste_cmd="${PASSWORD_STORE_paste_cmd:-xclip -o -selection "$X_SELECTION"}"

Don't introduce undocumented env vars like this.

> + local sleep_argv0="password store sleep on display $DISPLAY"
> + fi

You can rewrite the above as follows:

    if [[ -n $WAYLAND_DISPLAY ]]; then
        local copy_cmd=( wl-copy )
        local paste_cmd=( wl-paste -n )
        if [[ $X_SELECTION == primary ]]; then
            copy_cmd+=( --primary )
            paste_cmd+=( --primary )
        fi
        local display_name="$WAYLAND_DISPLAY"
    elif [[ -n $DISPLAY ]]; then
        local copy_cmd=( xclip -selection "$X_SELECTION" )
        local paste_cmd=( xclip -o -selection "$X_SELECTION" )
        local display_name="$DISPLAY"
    else
        die "Error: No X11 or Wayland display detected"
    fi
    local sleep_argv0="password store sleep on display $display_name"


> + local before="$($paste_cmd 2>/dev/null | $BASE64)"
> + echo -n "$1" | $copy_cmd || die "Error: Could not copy data to the clipboard"
> + local now="$($paste_cmd | $BASE64)"
> + echo "$before" | $BASE64 -d | $copy_cmd

These become "${copy_cmd[@]}" and "${paste_cmd[@]}" with quotes like that.

If you resubmit like that I'll happily merge.

Regards,
Jason


More information about the Password-Store mailing list