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

Trurl accept from stdin #288

Closed
wants to merge 3 commits into from
Closed

Conversation

jacobmealey
Copy link
Contributor

@jacobmealey jacobmealey commented Apr 21, 2024

this is a second pass at #277, I'm not sure what I was doing last time but the tests seem to be working now. I left the original -f - behavior unchanged, but with this I think could we could take it out as its redundant - but also its a pretty common pattern so I have no strong feelings either way about keeping -f - in.
Fixes #275

@emanuele6
Copy link
Collaborator

I don't like this at all

@emanuele6
Copy link
Collaborator

emanuele6 commented Apr 21, 2024

If I use trurl -g {host} -- "${urls[@]}", and stdin happens to not be a terminal, and urls is empty, my script now either gets stuck reading stdin or corrupts my stdin reading it. And the only thing I can do to avoid this is trurl -g {host} -- "${urls[@]}" </dev/null...

@jacobmealey
Copy link
Contributor Author

If I use trurl -g {host} -- "${urls[@]}", and stdin happens to not be a terminal, and urls is empty, my script now either gets stuck reading stdin or corrupts my stdin reading it.

I didn't think of that. shoot. having to redirect /dev/null is not good. I'm wondering if there is a way we can do this at all without getting into the situation described above. The issue really at hand is the implicit reading from standard which would be a really nasty bug to try and catch from a shell script point of view. The only way i see out of it would be using a command line flag... which gets us right back to -f.

If we close this PR we should also close #275 as well.

@emanuele6
Copy link
Collaborator

I don't think #275 is a good suggestion; I much prefer specifying -f -.

@jacobmealey
Copy link
Contributor Author

Okay, will close this PR.

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.

Read from stdin by default
2 participants