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

csplit: support reading from pipe #6951

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

fuad1502
Copy link

@fuad1502 fuad1502 commented Dec 12, 2024

Closing #6461

  • Removed metadata check, defer IO read error handling to InputIterator.
  • Use UResult instead of io::Result in InputIterator for a more uniform IO error handling.
  • Change error message on opening file to conform better with the original csplit.

@fuad1502 fuad1502 marked this pull request as draft December 12, 2024 06:19
@sylvestre
Copy link
Contributor

@fuad1502 any update on this PR ? thanks

@fuad1502
Copy link
Author

@sylvestre Hi, sorry for the long delay, I think I can handle the failing tests today 👍 There are some different behaviours in MacOS and Windows that I need to handle.

@fuad1502
Copy link
Author

@sylvestre I am having trouble with bringing up a MacOS virtual machine to debug and test the code on it. There might be some additional delay, sorry about this. In the mean time, do you know of any alternatives that I can try out other than running MacOS on VirtualBox / MacOS with Docker? I've tried both on my Ubuntu machine, but not yet successful.

Copy link

github-actions bot commented Jan 7, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@fuad1502 fuad1502 force-pushed the csplit-pipe-input branch 2 times, most recently from fad8e0a to 07eaffc Compare January 7, 2025 15:51
@fuad1502
Copy link
Author

fuad1502 commented Jan 7, 2025

MacOS issue fixed. It turns out echo with -n option is not supported in MacOS, I changed it to printf for maximum portability.

@fuad1502 fuad1502 marked this pull request as ready for review January 7, 2025 16:32
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

Successfully merging this pull request may close these issues.

2 participants