bugaevc / wl-clipboard

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

Provide a better error message when trying to pass --watch subcommand as an argument #59

Closed ghost closed 5 years ago

ghost commented 5 years ago

Would it be possible to make it so that if no argument to --watch is given, --watch just prints to stdout?

bugaevc commented 5 years ago

It would be possible, but it's exactly the same as spawning cat, so I decided you better write that out explicitly.

(Here's a little secret: the regular wl-paste actually spawns cat to print the clipboard data to stdout!)

bugaevc commented 5 years ago

If that works for you, please close this issue :wink:

ghost commented 5 years ago

It's weird to hear that it spawns cat even without --watch, yet in that case it works fine. :confused:

Unfortunately, it doesn't work when embedding wl-paste in another program. I use it like this. If I replace that command with gnu yes, my program will continuously receive "y", as expected, so it doesn't seem to be a problem on my part.

I thought, it must be that the output of spawned cat is not caught because it's a different process.

bugaevc commented 5 years ago

Must be because you're passing --watch=cat instead of --watch cat :wink:

Don't do that; --watch is supposed to take a long subcommand like so:

wl-paste --watch command args-to-command more-args

and I should make sure wl-paste prints an error if you try to do --watch=, so let's keep this issue open for that :slightly_smiling_face:

ghost commented 5 years ago

Yep! That, plus using -n (the latter was a bug in my logic). Thank'you!

bugaevc commented 5 years ago

Actually --watch mode automatically enables -n (I should document that!), so that cat won't get an extra newline

ghost commented 5 years ago

That might be what's still causing me problems. I'm still experimenting to see what's actually happening, and why.

EDIT: it's how I look for the next item coming (I look for a \n, which is wrong in various ways), so it's my bug, not yours

bugaevc commented 5 years ago

It looks like you're trying to read individual selections line by line, that's not going to work for multiline (and non-textual) selections, and that's part of the motivation why wl-paste --watch is designed this way.

Try either prefixing the selection with its length and then reading that much from the daemon, or have the command you spawn send you the full selection over separate "connections" — perhaps by connecting to a socket that your daemon listens in it something.

ghost commented 5 years ago

I managed to make everything work :sparkles: Now I'm testing, seems good so far. Thank'you for your assistance! Feel free to close this issue when you wish.

bugaevc commented 5 years ago

This looks like it's still incorrect. There's no guarantee that the contents that get output by one invocation of cat would be read in one invocation of read() on your daemon's side, nor that it won't happen the other way around. It happens to work most of the time (and probably is fine for now), but it's not reliable.

Again, the correct way to make this work is explicitly delimiting different "paste"s, either in one channel/pipe by doing something like prefixing each paste with its byte length or by using multiple channels/sockets/pipes. Both unfortunately require you to switch from cat to something more complex.

You can make a separate script, or you could actually just add a new subcommand to clipman main executable (e.g. clipman append) that would read its stdin and append it to the history file, and then use something like

exec.Command("wl-paste", "-t", "text", "--watch", osext.Executable(), "append")

on the other hand, there's not much point in doing this if the file you're storing the history into itself is in plain text. Perhaps just cat is an okay "easy" way of handling it then.

ghost commented 5 years ago

I'll probably rework clipman to be used like this: wl-paste -t text --watch clipman store, ie as a oneshot filter & storer rather than a demon

bugaevc commented 5 years ago

Yeah, that's basically what I'm suggesting 😀

And indeed you don't even have to provide your own daemon wrapper, wl-paste can now work as daemon itself. Or maybe you do want it to do some options preprocessing or something. It's up to you to design how this would work, wl-clipboard just provides convenient building blocks 😀