bugaevc / wl-clipboard

Command-line copy/paste utilities for Wayland
GNU General Public License v3.0
1.62k stars 59 forks source link

Add --print0 option #93

Closed gandalf3 closed 1 year ago

gandalf3 commented 4 years ago

This option causes wl-paste to emit a null character when the clipboard is clear (or cleared, in the case of --watch).

This would allow a clipboard manager running as the argument to wl-paste --watch to detect when the clipboard has been lost, and re-fill it accordingly. See https://github.com/yory8/clipman/issues/43#issuecomment-687583506 for some discussion.

bugaevc commented 4 years ago

Hi!

I agree that persisting the selection after a client exits is an important case to support. However, this solution you propose is not a great one, for the following reasons:

In fact, I believe the existing Wayland protocol(s) are not enough to provide robust, reliable clipboard persistence (if the clipboard manager is to be implemented as another client, not inside the compositor), and we need to extend the protocol further if we want to make it happen. I've argued this point in more details in this discussion, please see that.

gandalf3 commented 4 years ago

Wow it seems I've stumbled into quite the rabbit hole! Thank you for that rundown, you are right this solution is not an optimal one. I'd still like to make an argument for accepting it:

Even if wlroots does implement improvements to the clipboard protocol (I hope it does), how would you propose to notify --watch arguments about clipboard lost/cleared events? There could perhaps be an option to pass arguments signaling those events, which would at least allow nil/empty/null character selections to be distinguished by the -w program. But that could also be done now, even in the absence of any protocol improvements. Admittedly it is a bit weird, and an decided increase in complexity for anything running as the -w argument under such a new option.

I personally am quite fine with sacrificing the ability to copy/paste single null chars for scriptable clipboard management functionality on par with X11. I can confidently say I have never tried to copy/paste a solitary null char before today!

In my testing single null chars don't work consistently with various X11 clipboard utilities:

After echo -ne '\0' | xclip -selection clipboard -in:

After echo -ne '\0' | xsel -ib:

Not that the functionality of X11 is really what we should be striving for, but for the purposes of scripting, is the loss of this one corner case really too terrible to not support persistence at all? It won't affect any other uses of wl-paste; a really intrepid script could even call wl-paste without -0 upon receiving a null char to deduce if it's a real null char or if the clipboard is in fact nil.

However I understand if you feel that this --print0 option muddies the waters too much, in which case I'm curious how you would envision signaling the -w program in a more unambiguous way.

Thank you for pushing to give us clipboard management fit for the future!

bugaevc commented 4 years ago

Hi again (and sorry for the late reply — I've started writing it, then restarted my browser, and it apparently decided to just forget everything I've written. Ah, the wonders of a web UI.)

I am 100% with you that it would be nice if wl-paste --watch communicated nil selections to the command too, somehow. The feature just feels incomplete without that, and resolving this has long been on my ~to-do~ potential future improvements list.

But representing nil selections as a single null byte just doesn't feel right. You're right that any confusion would be rare in practice, and avoidable if the spawned command is careful enough to check wl-paste a second time upon reading a single null byte (though I bet you no one would care enough to implement that).

But it still just is not right. Consider this. In GNU find, --print0 is used to separate file paths with a null byte. File paths are arbitrary byte sequences (they can include b'\n' in particular), except that they cannot include the null byte, so it just makes perfect sense to use it as a separator for those paths. But Wayland selections are not like that, they can be arbitrary binary data (for each MIME type, too), or nil. It's as if --print0 instructed GNU find to print a single null byte if no files were found that matched the criteria. I think you'll agree with me that that doesn't make as much sense, and is not nice.

Now, I have considered other potential ways that a nil selection could be communicated; here are just a few that come to mind:

None of them feel right, and that's why I haven't implemented any, instead deciding to ship --watch without a way to get notified of nil selections, at least for now. If you can think of a way that passes my informal test of "feeling right", I'd be happy to implement & ship it.

gandalf3 commented 4 years ago

No problem, I hate when that happens :)

I think you've probably considered all the possible "nice" ways, though I will throw the possibility of an environment variable (for example, WL_PASTE_CLIPBOARD_STATE='CLEARED') onto the end of your list; it seems a little less intrusive than an argument (programs which already parse their arguments won't be broken) and is much more explicit about its meaning than doing something weird with stdin.

maximbaz commented 3 years ago

Hi guys! Oh wow I also did not expect what a rabbit hole this is 😅

First things first, after reading all the attached discussions, I agree with you @bugaevc that the existing protocol is certainly not enough to provide a robust experience, and in an ideal world things need to be improved on a deeper level.

Having said that, how do you guys feel if we try to find a pragmatic solution to use - not the ideal one, but something that could serve us well for the "most" cases?

It is true that if we pass whatever signal of a NULL offer to a script, there is no way to distinguish if an app was closed or the selection was cleared intentionally - but that is a protocol issue, it's not the fault of wl-clipboard. Whatever the meaning, let us propagate this NULL offer down to the clipboard manager, and let it deal with it.

As for the implementation, I kinda share the sentiment above that \0 doesn't feel right, and although, yes, none of the alternative proposals are ideal, I would personally go with the "Spawn the command with stdin attached to /dev/null" one - it's simpler, it doesn't require an extra flag to wl-paste, and we can all agree and document that this is what currently happens until wayland provides a better protocol.

Passing any indicator of the NULL offer is in any case better than passing no information whatsoever...

What do you guys say?

bugaevc commented 3 years ago

I tend to agree, actually.

There clearly isn't enough interest in collaborating on a proper protocol from the wlroots or KDE sides. So we should make the best of what we have.

I actually like the environment variable approach suggested by @gandalf3 above better than any of my own ideas, because environment variables are specifically meant for situations like this, where we need some kind of a side channel. And since an environment variable lets us pass a string value (and not just a boolean, like whether stdin is /dev/null or not), we can make this forward-compatible with a potential future dedicated Wayland protocol. For example,

(bikeshedding welcome!)

I presume that selection(nil) events due to a client exiting are more common than explicit clears, and losing a selection due to an existing client is more painful than having a password manager unexpectedly save/persist your password, so for now (while we can't distinguish them) wl-paste --watch would set CLIPBOARD_STATE=nil (as opposed to clear) when it gets selection(nil).

Finally, we can combine environment variable approach with passing /dev/null as stdin.

What do you think? @YaLTeR, @yory8, would love to hear your thoughts too.

maximbaz commented 3 years ago

I agree to all - it's a future-proof solution, even though there is no promise that the protocol will ever be extended beyond the existing binary alternative.

Let's do this!

Finally, we can combine environment variable approach with passing /dev/null as stdin.

Sounds like a good idea as well, so that we don't suddenly introduce any crashes to scripts that don't know about the new environment variable and simply read the passed stdin as is.

bugaevc commented 3 years ago

Sounds like a good idea as well, so that we don't suddenly introduce any crashes to scripts that don't know about the new environment variable and simply read the passed stdin as is.

To be clear, wl-paste --watch is intended to work not only with scripts/programs written specifically for it, but with any command (provided it expects to read data from its stdin). For example, you might use curl --data-binary @- http://paste.example.com to upload your clipboard contents to a pastebin. So of course, anything we come up with here must still work if you're just reading stdin and have no idea about any clipboard or whatever.

It's also likely a good idea to not mention Wayland or wl-clipboard by name in the environment variables. This way other clipboard access utilities (including xsel/xclip, should they decide to implement a watch feature) would be able to implement the same interface, and any scripts/programs written for this interface will be automatically interoperable with them.

ghost commented 3 years ago

I love this idea of environment variables!

The CLIPBOARD_STATE=sensitive value you suggest would allow a compliant clipboard manager to also fix the problem of passwords, which currently requires similar workarounds (I use lockfiles in /tmp, hadn't thought of setting an env). EDIT: I mean, if wl-copy -c sets it! EDIT2: do we really need to distinguish between clear and sensitive?

bugaevc commented 3 years ago

do we really need to distinguish between clear and sensitive?

The idea was that you'd set sensitive with the sensitive data (when you copy your password) and clear once the clipboard has been cleared (after the sensitive data). This way, a clipboard manager would be able to ignore the password in the first place, or treat it specially otherwise, before it even receives the clear event.

But in any case, currently nothing like that exists in Wayland clipboard protocols. And if such a protocol is ever designed, there'd be a comprehensive evaluation of how this should work and whether we need separate flags for this and that.

EDIT: I mean, if wl-copy -c sets it!

Assuming these protocols existed, wl-copy -c would set the appropriate flags so that the watch command gets clear, and there'd be a separate flag, say wl-copy --sensitive < my-password.txt, to set the sensitive flag (the watch command gets sensitive).

But again, all of this is just a speculation about what could be possible. With what we have currently, there'd be just data and nil.