search

CopyText / TextCopy

A cross platform package to copy text to and from the clipboard.
MIT License
513 stars 41 forks source link

Configuring the process timeout on BashRunner #830

Open waf opened 7 months ago

waf commented 7 months ago

Thanks for creating TextCopy, it's a great library. I use it in my CSharpRepl project, and it's worked well.

Is the feature request related to a problem: Yes

I'd like to be able to configure the process timeout in BashRunner. I recently had a bug report from a user about a timeout on Linux (ref https://github.com/waf/CSharpRepl/issues/327), and I see the process timeout is set to 500ms in BashRunner:

https://github.com/CopyText/TextCopy/blob/701c64408f75a7a5a8f4b0328372aca643a69c06/src/TextCopy/BashRunner.cs#L48

For reference, the stack trace reported by the user was:

System.Exception: Process timed out. Command line: bash -c "cat /tmp/tmpWDKMZj.tmp | xsel -i --clipboard ".
Output: 
Error: 
   at BashRunner.Run(String commandLine) in /_/src/TextCopy/BashRunner.cs:line 32
   at LinuxClipboard.InnerSetText(String tempFileName) in /_/src/TextCopy/LinuxClipboard_2.1.cs:line 42
   at LinuxClipboard.SetTextAsync(String text, CancellationToken cancellation) in /_/src/TextCopy/LinuxClipboard_2.1.cs:line 22
   ... stack frames from application ...

I'm happy to adjust my application's usage if there's a better way of handling this.

Describe the solution

I can provide a PR for this feature, but there are a few different options for implementing it and I'm looking for guidance:

  1. I could support it only for the async APIs, and then use the CancellationToken that's already there to control Process.WaitForExit (moving to Process.WaitForExitAsync).
  2. I could provide some optional configuration option that would be passed to the SetText method (and possibly GetText, just for parity?). Either passing just a timespan/int timeout, or some ClipboardConfiguration class.
  3. Similar to above, but support passing the configuration option to the Clipboard's instance constructor instead.

In my opinion, Option 1 is the best, but Option 2 or 3 would be good if there are other configuration scenarios you'd like to support. Thanks again for creating such a useful library.

SimonCropp commented 7 months ago

You seem to have a good handle on the problem. How about you implement the approach you see as best.