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.
