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

Complete cat command #55

Closed
wants to merge 1 commit into from

Conversation

tacoda
Copy link
Contributor

@tacoda tacoda commented Aug 10, 2022

Closes #54

Copy link
Owner

@faraazahmad faraazahmad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Although I found one small issue

If you run it with multiple files and one of those doesn't exist, it differs from coreutils in erroring out.

For example, run the following and compare results between cat in shellrb and cat from coreutils

cat Gemfiles Gemfile.lock Gemfile

This will fail on the first file and not proceed further but coreutils will report error and then move onto the next params

cat Gemfile Gemfile.lock Gemfiles

This will silently fail here but coreutils will report an error,

I think you'll have to write an extra test for that scenario, but great work! I really appreciate it!

@tacoda
Copy link
Contributor Author

tacoda commented Aug 24, 2022

NOTE: I may close this PR depending on the decision made in #57

@tacoda
Copy link
Contributor Author

tacoda commented Sep 9, 2022

Closing due to the comment above. We may or may not pull out this functionality to a separate coreutils gem.

@tacoda tacoda closed this Sep 9, 2022
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.

Complete remaining cat functionality
2 participants