Skip to content

Conversation

@likhithabz55
Copy link

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

@HadyTinawi
Copy link

Verification

Issue:
“For my project, I want to implement uniq. It should read lines from a file (or STDIN), remove adjacent duplicates, and support flags like -c, -d, -D, -i, -u, and -s (skip chars). The utility should also handle combined flags correctly and reject contradictory ones.”

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:
parseFlags works but nests a lot of else if. Consider a switch-style structure or even a table of flags → options. Not urgent, just makes it easier to maintain if more flags get added.

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.
Two polish suggestions:

Maybe refactor parseFlags to be less nested for maintainability.

Otherwise, this is good to go 👍

@spganti
Copy link

spganti commented Oct 6, 2025

High Level Checks:

  • [✓] PR fully resolves the task/issue
  • [✓] Does the PR have tests?
  • [✓] Does the PR follow project code formatting standards?
  • [✓] Does the OS still compile and run?
  • [✓] Is documentation included?

Code Checks:

  • [✓] Does the code achieve its intended purpose?
  • [✓] Were all edge cases considered?
  • [✓] Is the code organized and maintainable?
  • [✓] Does the code maintain the same level of performance? (no performance regressions)

Verification:
The issue filed for this project was to implement the functionality of uniq command. It compares adjacent lines in a file or standard input and filters out the duplicate lines and writes the result to standard output. It retains the first occurrence of each sequence of identical adjacent lines along with -c, -d, -D, -I, -u, -s flags. Everything was implemented that was discussed in the issue.

Testing:
I followed along with the testing documentation given in the uniq.md and everything worked as expected and described. Most of the edge cases were accounted for so I didn’t find any problems while testing.

Code Walkthrough:
In the uniq.md you talk about conflicting flag types (ex: -c -u), I think that should be handled in your ParseFlags function and have an error printed.

https://github.com/likhithabz55/FogOSv2/blob/a10d6cf56755b015c34e344e309847b2c4a336de/user/uniq.c#L162-L186

Formatting:
The code was formatted well and everything was consistent in the program. The code style (usage of structs and flags) matched the conventions in the given code.

Overall, really cool and useful command line utility to make and you implemented it in a readable and clean way!

@yifanwane
Copy link

yifanwane commented Oct 11, 2025

  1. Overview

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.

  1. Verification & Testing

Test Inputs:
empty.txt, test1.txt, test2.txt, test3.txt, and a missing file does_not_exist.txt.

Results Summary:

Empty file (uniq empty.txt) No output produced ✅
Empty file with flags (-c -u -i -s -D) No output produced ✅
Missing file Graceful error No such File or directory!! ✅
test1.txt Basic duplicates handled correctly ✅
test2.txt Case/skip options work as intended ✅
test3.txt Duplicate and count logic correct ✅
Invalid flags (-z, -s banana) Proper error expected ✅
Performance test (large file ≈ 100k lines) No crash or slowdown ✅

All outputs match expected results; behavior is consistent with GNU uniq semantics.

  1. Code Walkthrough & Logic

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.

  1. Formatting & Documentation

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.

  1. Final Checklist
    High Level Checks

✔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.
It passes all functional and edge-case tests and shows robust error handling.
Formatting and documentation meet CS 326 standards.

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.

4 participants