-
Notifications
You must be signed in to change notification settings - Fork 49
find command #89
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
base: main
Are you sure you want to change the base?
find command #89
Conversation
|
I can code review this. |
1 similar comment
|
I can code review this. |
|
I can review this! |
|
I can also review this |
|
Hi! This is a great implementation of a basic find command. Well done! The PR meets the core functionality outlined in the description. I liked the use of flags like
High Level Checks:
Code Checks:
Overall, superb job! Thank you. |
|
The PR complies with all the posted specifications for the find command. The code is nicely streamlined and there is not much requiring fixing. Suggestions: JavaDocs: add more information for the header comment in Add more comments within functions to clarify what each block is doing such as the while loop in In this if statement (line 80-84): // if it's a directory, traverse it
if (st.type == T_DIR) {
if (strlen(path) + 1 + DIRSIZ + 1 > sizeof(buf)) {
printf("find: path too long\n");
return;
}close Add more test cases such as more complex patterns to match for the name flag (e.g. a mix of Print an error message/usage when the syntax of the Add a help flag such as
|
|
High Level Checks: Code Checks: Hi! The code implementation achieves the suggested purpose specified in the PR outline. While having a good description of how to test the PR as well as the current limitations, the formatting is good and documentation is included. Great job! A few suggestions:
This may seem redundant but in agreement to the other reviewers I also agree that another approach to testing could have been to provide test directories and nested test directories into FogOS-find: Overall, amazing work! |
|
This is really nice, self contained, and perfect for P1. I don't have any reservations merging this because it's a great start. There are some things that I ran into that were surprising coming from the usual find command:
I like the support for checking file / dir types. As mentioned in the other CRs, I think the only major issue here is with how command lines are handled. As is, it's pretty brittle and expects things to be input in the exact order or it won't work, and there's no help info supplied (and nothing added to /docs to explain either). Instead of hard coding the argument positions, I'd handle them up front by searching for args that begin with Overall this is minor, you can earn +0.25 for implementing improved arg parsing. Overall very well done. |
Teammate: Esha Dupuguntla @eshadupz
Find command
Description
The find command prints out the path of a file or directory based on the start of a directory given within the command. Syntax to search for a file is formatted like this:
find <flag/option> <pattern/name of file or directory>
Additional flags
Changed files
Limitations
Example run in make qemu to test
make qemu
$ mkdir /testdir
$ echo "test" > /testdir/sample.txt
$ find /testdir -name sample.txt
other commands that can be run using find:
$ find . -name tester.txt
$ find . -name *.txt
$ find /testdir -name sample.txt (double check that testdir exists)
$ find . -type d
$ find . -type f
$ find /testdir -type f