Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuring the process timeout on BashRunner #830

Open
waf opened this issue Mar 23, 2024 · 1 comment
Open

Configuring the process timeout on BashRunner #830

waf opened this issue Mar 23, 2024 · 1 comment

Comments

@waf
Copy link

waf commented Mar 23, 2024

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 waf/CSharpRepl#327), and I see the process timeout is set to 500ms in BashRunner:

var result = process.WaitForExit(500);

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants