bugaevc / wl-clipboard

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

Also close stderr file descriptor #110

Closed caioraposo closed 3 years ago

caioraposo commented 3 years ago

This is similar to aa4633b8 but for stderr.

It can be reproduced with the following shell pipe line which does not terminate.

echo "text to copy" | wl-copy 2>&1 >/dev/null | cat

This fixes an issue I was having with a text editor.

bugaevc commented 3 years ago

Hi, and thanks for taking interest in wl-clipboard!

Unfortunately, this case is less clear-cut than https://github.com/bugaevc/wl-clipboard/issues/100. An important point there was that wl-copy never writes to its stdout (as it produces no output), nor does it read from its stdin once it has read the whole stdin once (and copied it into a temp file). But it may and will use stderr for logging any errors, at any time, including after forking into background.

So redirecting stderr to /dev/null, as you suggest, is basically silencing any errors wl-copy may encounter. I believe it would also break WAYLAND_DEBUG logging. If silencing any errors/warnings from wl-copy is what you actually want, why don't you run it with stderr attached to /dev/null to begin with, e.g. wl-copy 2>/dev/null?

In case of an editor using wl-copy internally to implement copying, the proper fix would be for the editor to consider the text copied once the parent wl-copy exits, but keep watching for potential errors (and perhaps displaying them to the user) until the pipe is closed. Note that wl-clipboard is not the only clipboard program to use background-forking, e.g. xclip does the same, so the fix wouldn't specifically target wl-clipboard either.

caioraposo commented 3 years ago

No problem, I clearly misunderstood the issue. I'll close this PR but thanks for your suggestions!