-
-
Notifications
You must be signed in to change notification settings - Fork 556
Add grep test data and update readme #327
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
Conversation
|
||
The `grep` command should support multiple flags at once. | ||
|
||
For example, running `grep -l -x "hello" file1.txt file2.txt` should |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either the -x
should be a -v
, or the do not contain the string "hello"
should be changed to match what -x
does
42fc966
to
c098134
Compare
@petertseng I've incorporated all your feedback (thanks by the way!) and updated the PR. |
|
||
1. The regular expression used to match lines in a file. | ||
2. Zero or more flags to customize the matching behavior. | ||
3. One of more files in which to search for matching lines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of -> or
Everything seems to be in order in the test data. I have a small fix to the README and then I think it will be OK to go. One last thing I see is that the README says the pattern can be a regular expression but the tests don't have things like |
c098134
to
4229ea0
Compare
The README has been fixed, and I've reworded the text to not mention regular expressions as that is indeed outside the scope. |
👍 from me |
The current
grep
exercise is a bit odd. If mentions that the user should implement a function that returns pairs of line numbers and matching lines. However, this does not easily fit the latter suggestion that thegrep
function should also support flags, of which one flag is intended as only listing file names.In this PR, I've reworded the exercise to make it more like you're re-implementing the Unix
grep
command, as now it expects just a string to be output (which should correspond to the string output by thegrep
command).Furthermore, I've added a suite of test cases.