[PATCH] RFC: hooks for clipboard manager integration

[Allan Odgaard]

On 6 Jul 2020, at 1:03, Dov Feldstern wrote:

> I'm not sure exactly what you mean by a "callback" here: a specific
> implementation of how exactly to do the copy and paste (which the
> current code seems to *almost* support, in the form of "copy_cmd" and
> "paste_cmd", all that's missing is a means for changing them less
> intrusively than by adding more if cases in the code itself)? Or
> remiplementing the entire clip() function as a whole?

I was talking more in the abstract :)

But yes, I think the practical implementation would be for a simple way 
to fully replace the clip function, without having to send patches 
upstream, but of course also with a more clearly defined “contract”.

An implementation could be something like this:

     for file in "$PASS_DIR"/override.d/* /etc/default/pass 
"${XDG_CONFIG_HOME:-$HOME/.config}/pass/default"; do
       [[ -x "$file" ]] && . "$file"
     done

     ⋮

     ${clip_function:-clip} $password $duration

Distribution ("$PASS_DIR"/override.d/) could define multiple clip 
functions and set $clip_function to the one appropriate for the 
platform, and user could change that to either an alternative function 
from the distribution, or define their own clip function and set that to 
be used (either globally via /etc/default or locally via ~/.config).

> IIUC, the latter can already be achieved by providing a
> platform-specific implementation of clip()?

Yes, the problem currently is only that adding support for clipboard 
manager by providing new clip override requires patching the existing 
source; so this part should be made overridable.

> The problem with reimplementation, though (and why I ultimately
> preferred to use hooks), is that there is non-trivial logic going on
> in the function, which might be lost by the reimplementations. For
> example, the sample snippet you provide above doesn't deal with the
> subtlety of avoiding a second call to 'pass -c' restoring

I do not directly disagree, however, the non-trivial logic boils down to 
running sleep in a subprocess with a special name so that pass can kill 
it.

This is not complex (one line), but I agree that it would be nice to 
“abstract” it because there are some hardcoded assumptions about how 
to name the process, furthermore, the current implementation does not 
look ideal, as pkill (called by pass, which would still be outside the 
clip function) is followed by sleep 0.5, which is to give time for the 
clip function to restore the clipboard.

It would be good to get this replaced with a proper wait (for old clip 
function/process to exit): Especially on macOS, this delay could be too 
short, as Apple in their infinite wisdom have littered the system with 
“code signature verification” that does online certificate 
revocation checks that have a 3s timeout, so these days, it is sadly not 
uncommon to see delays from low-level APIs of 0.5 seconds and more.

What I was originally thinking was to provide a “sleeper function” 
to the clip override, e.g. something like this:

     function user_function {
        password="$1"
        sleeper="$2"

        old="$(pbpaste)"
        printf "%s" "$password" | pbcopy

        $sleeper

        if [[ "$(pbpaste)" == "$password" ]]; then
           printf "%s" "$old" | pbcopy
        fi
     }

I was originally rejecting this idea, because it requires bash code 
“in the middle” of the function, e.g. for macOS I was thinking of 
writing the entire clip function in Swift or Objective-C (necessary to 
tag clipboard content and restore non-text data), but possible could do 
it with a pre-sleep and post-sleep argument passed to the compiled 
function, the downside is it then needs external storage for old 
clipboard content, but it avoids introducing a “contract” for how to 
abort

An alternative would be to also make the pkill a user override, so for 
macOS I could have this in ~/.config/pass/default:

     clip_function=/path/to/pass_helper clip
     abort_function=/path/to/pass_helper abort

Calling `pass_helper clip …` would setup SIGTERM handler, and calling 
`pass_helper abort` would be responsible for sending SIGTERM and wait 
for the process to terminate.

I am leaning toward the simpler $sleeper argument though, so 
~/.config/pass/default would contain:

     function macos_clip_function {
        password="$1"
        sleeper="$2"

        /path/to/pass_helper copy "$password"
        $sleeper
        /path/to/pass_helper restore
     }

     clip_function=macos_clip_function

> […] Although, perhaps for a platform-specific implementation it 
> makes more sense to expect a high-quality implementation […]

Yes, although an attempt should be made to simplify the interface for 
platform-specific implementations, that was really what I was aiming for 
with my reply: Allow overrides to be provided by users, simplify and/or 
document the contract, provide helper functions if necessary (or use 
higher order functions), etc.

And if you still think hooks make sense: provide a “platform 
specific” override that uses hooks. So a user can put this in 
~/.config/pass/default:

     clip_function=xclip_with_user_hooks

And it will use xclip for read/write, but call user hooks before/after 
touching the clipboard.