-
Notifications
You must be signed in to change notification settings - Fork 25
project 1 : uniq command implementation #36
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?
Conversation
|
Verification Issue: The uniq implementation meets most of the requirements. It parses the right flags, prints results based on options, and handles edge cases like empty files and invalid flags. Contradictory flags also print an error or exit cleanly, which is great. Testing I walked through the provided test files (emptyFile.txt, test1.txt, test2.txt, test3.txt). The expected outputs match the described behavior. For example, uniq -c test1.txt correctly reports counts. uniq -i test2.txt merges case-insensitive duplicates. uniq -D test3.txt prints all duplicate lines. So the base functionality is solid. The only area I’d push further is case handling (to_lower) which works fine but could be simplified using tolower() if available, and large-line edge cases (currently capped at BUF_SIZE). Code Walkthrough Great stuff y’all. A quick thing I noticed: Flag parsing logic is clean but a little dense: Everything else (buffer reading, equals comparison with ignore-case and skip-chars, printing logic in print_line) looks correct and well-structured. Formatting The formatting is consistent, indentation is clean, and the style matches other user-space utilities. No major nits here. High Level Checks: [x] PR fully resolves the task/issue [x] Does the PR have tests? [x]Does the PR follow project code formatting standards? [x] Does the OS still compile and run? [x] Is documentation included? Code Checks: [x]Does the code achieve its intended purpose? [x]Were all edge cases considered? [x]Is the code organized and maintainable? [x]Does the code maintain the same level of performance? TL;DR Implementation is solid — it handles flags, contradictions, and test cases correctly. Maybe refactor parseFlags to be less nested for maintainability. Otherwise, this is good to go 👍 |
|
High Level Checks:
Code Checks:
Verification: Testing: Code Walkthrough: Formatting: Overall, really cool and useful command line utility to make and you implemented it in a readable and clean way! |
The uniq utility correctly implements all required features described in the assignment and README: Reads input from STDIN or a file. Removes adjacent duplicate lines. Supports flags: -c, -d, -D, -i, -u, -s N. Handles combined flags and provides proper error messages for invalid or missing files. Verified to compile and run inside the xv6 environment.
Test Inputs: Results Summary: Empty file (uniq empty.txt) No output produced ✅ All outputs match expected results; behavior is consistent with GNU uniq semantics.
Structure: Straightforward read–compare–write loop using fgets() with proper string comparison. Flag parsing: Handled cleanly via conditionals; mutually exclusive flags checked. Edge handling: Correct EOF detection and file open validation. Readability: Variables and control flow are clear and consistent with CS 326 style guide. Error handling: Gracefully prints messages for invalid flags and missing files. Minor style suggestions for future improvement: Add short header comments per function (e.g., purpose and parameters). Group error messages into a single usage() function for cleaner structure.
Indentation and brace style follow course standards. No trailing whitespace or mixed tabs/spaces. Console messages are concise and user-friendly. Documentation included in README with flag explanations and examples.
✔PR fully resolves the task/issue ✔Does the PR have tests? (Comprehensive functional and edge tests run) ✔Does the PR follow project code formatting standards? ✔Does the OS still compile and run? (make qemu succeeds) ✔Is documentation included? (README and examples present) Code Checks ✔Does the code achieve its intended purpose? ✔Were all edge cases considered? (Empty, missing, invalid flags tested) ✔Is the code organized and maintainable? ✔Does the code maintain the same level of performance? (no performance regressions observed) Summary The uniq implementation in FogOS is complete, correct, and maintainable. |
This Pull requested is created to address issue #14 . It implements uniq command with all the functionality mentioned in the issue.
The PR :
- Fully resolves the issue
- Includes documentation for how to build and run tests and how to compare with expected outputs
- Includes test files
- OS will compile and run