extended wc

[natyhl]

Natalie and Paing - We extended the wc.c to consider multiple flags. Documentatation can be found in docs/wc.md

[ebeckk]

I will be reviewing this PR.

[likhithabz55]

I'll be reviewing this PR

[likhithabz55]

Verification :

Issue
The wc program prints statistics about its input files. By default, it displays all counts with labels. You can use flags to select which counts to display:

• -l : Line count (number of \n newlines)
• -w : Word count (sequences of non-whitespace: not space, tab, carriage return, newline, vertical tab)
• -c : Character count (counts every character, including newlines and tabs; ASCII expected)
• -b : Byte count (same as -c on xv6)
• -L : Max line length (length of the longest line, including its newline)
  If multiple flags are given, each selected count is displayed with a label.

The implementation is solid and covers all the requirements mentioned in the documentation.

Testing :

I tried running the tests provided in the documentation along with some tests of my own. All the tests are returning expected outputs. The code is able to handle all edge cases correctly.

Code Walkthrough :

Great work!! The implementation looks clean and solid. the code is well structured. I only have a few suggestions to improve the maintainability

1. Right now, the wc_stats method inside wc.c is a bit long. I would suggest splitting it into two different methods. One part could deal with only the core logic for wc (loading lines into buffer, finding max length, num of lines, word count etc) and the other part could deal with printing results to console.

2. Also, the current logic for printing results based on multiple flags looks too nested. I would suggest using switch statements instead of if-else.

3. I noticed that you have listed wc as a system call inside user.h. There was an entry inside syscall.c and the implementation logic inside sysproc.c. However you seem to be implementing it as a utility function instead of as a system call as I couldn't find the syscall methods being called from inside wc.c (Maybe I missed something. Please ignore in that case) . I believe your implementation is the right way to do this and would suggest you to remove the system call entries and implementations from the kernel files and user.h if you are not using them

Documentation :

The documentation is present and consists of detailed description of how to build and run the test cases.

Formatting :

The formatting is consistent and the code is indented. I'd suggest to add comments above the methods in wc.c to make it easier to understand their functionality

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? (no performance regressions)

Overall, the implementation is solid and covers all edge cases. Maybe refactor wc_stats to improve readability and remove the system call entries.

[jadecuster]

I'll be reviewing this pr request

[jadecuster]

Verification :

Issue
With no flags, all counts are displayed with labels. With flags, only the
requested counts (plus the file name) are shown:

• -l : Line count (number of '\n' newlines)
• -w : Word count (sequences of non-whitespace: not space, tab, CR, LF, VT)
• -c : Character count (counts every byte read; ASCII expected on xv6)
• -b : Byte count (same as -c on xv6)
• -L : Max line length (includes the newline in the length)

Testing :
I ran the test cases and all passed. I tried some other cases and it looks pretty good. I noticed that if you run it with a flag that doesn't exist it just runs a standard wc. I might just suggest adding in something that makes it if an invalid flag is typed in that it returns a "invalid flag" error.

Code Walkthrough :
Code looks pretty good and very well organized. Try to work on making things more easily readable though.

Documentation :
Documentation in wc.md is great. I would suggest adding more comments in wc.c just so it's more readable and easier to manage.

Formatting :
Everything feels really squished. There just isn't much line breaks so it feels a little harder to read but that's minimal.

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? (no performance regressions)

Great project though, and your implementation was exactly what you were trying to achieve . Great job!

[ebeckk]

Verification:

Issue
The wc program prints statistics about its input files. By default, it displays all counts with labels. You can use flags to select which counts to display:

-l : Line count (number of \n newlines)
-w : Word count (sequences of non-whitespace: not space, tab, carriage return, newline, vertical tab)
-c : Character count (counts every character, including newlines and tabs; ASCII expected)
-b : Byte count (same as -c on xv6)
-L : Max line length (length of the longest line, including its newline)
If multiple flags are given, each selected count is displayed with a label.

Testing:
All of the test cases provided ran correctly and passed. Additionally, when trying different tests (files with different amount of words, punctuation, etc) they ran well. Empty files pass as well.

In terms of edge cases/logical errors, I wasn't able to find any there. There are a few small things but I wouldn't consider them errors. I'll explain them in later seciton.

Code Walkthrough/Formatting:

1. When a flag isn't used, it runs all of the flags. Maybe instead of everything being ran, just the word count could be enough if no flag is inputted.
2. When an incorrect/non-existing flag is used, the same behavior happens. For example, if I do "-z", it still runs. I suggest that maybe throw an error here or a print statement saying that the flag doesn't exist.

Go through the code once to get an idea of what it’s doing.

Run any test cases and verify they pass. Try adding new inputs / tests to see if you can break the code. Look for values that are hard-coded that may not need to be.

• In terms of documentation for the code, there needs to be more comments in wc.c. There is only one comment in the whole file which isn't enough. With more comments, the code will be more readable and easier to understand.
• Additionally, instead of having a bunch of nested if statements when checking for flags, it'll look more readable and more structured if you use switch statements instead.
• Overall, the documentation in /docs is great and the formatting looks good too. Just a few small changes for code readability should do it.

Checklist
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)

[malensek]

Looks good to me. I am not personally a fan of the very verbose output, but maybe that's because I'm old and expect Unix utilities to treat me like they hate me. So no problem there.

Ok, not sure why these reviewers all felt the need to copy and paste the readme, I felt like I was losing my mind here. Maybe... oh, never mind. There is no -c option for wc in xv6. I'm not sure why your readme says that or why these reviewers are parroting it.

Fixes:
You should definitely remove the extra system call that is unused and handle invalid flags, and when you have errors you should print to stderr instead of stdout. -> +0.25

4.75/5