Birmingham | 26-JUL-SDC | Merve Reis | Sprint 3 | Sheel Tools - #614
Birmingham | 26-JUL-SDC | Merve Reis | Sprint 3 | Sheel Tools#614mervereis wants to merge 2 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
mervereis
added
the
Needs Review
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start but there is still a lot to do here.
Is your way of handling options/arguments the best? Is there a better way of doing this?
|
|
||
| if (numberNonBlank) { | ||
| if (line.trim() === "") { | ||
| process.stdout.write(output); |
There was a problem hiding this comment.
When I am looking at the output of blank lines I get some odd formatting, do you see this in your testing?
There was a problem hiding this comment.
Yes, I found the formatting issue in my testing. It was caused by the trailing newline being treated as an additional blank line. I fixed it so blank lines are handled correctly, and the output now matches the standard cat command. @LonMcGregor
| } | ||
| } | ||
|
|
||
| if (!onePerLine) { |
There was a problem hiding this comment.
What is this last empty log doing?
There was a problem hiding this comment.
It adds the final newline after the filenames have been written with process.stdout.write(), which does not add one automatically. The !onePerLine condition ensures this only happens in the default format, where filenames are displayed on the same line. @LonMcGregor
|
|
||
| if (filesCounted > 1) { | ||
| let result = ""; | ||
| if (countLines) { |
There was a problem hiding this comment.
This printing code looks to be repeated, do you think you could reduce the duplication somehow?
There was a problem hiding this comment.
Yes, I reduced the duplication by moving the repeated output-building logic into a shared formatResult() function. Both the individual file results and the final total result now use this function, keeping the formatting consistent and easier to maintain. @LonMcGregor
| } | ||
| result += "total"; | ||
|
|
||
| console.log(result); |
There was a problem hiding this comment.
When you have multiple files, is the output formatting as good as it could be? How could it be made neater?
There was a problem hiding this comment.
The output could be neater. The numeric values should be right-aligned in consistent columns, matching the standard wc format. I updated the formatting so both individual file results and the total row use aligned columns, making the output easier to read. @LonMcGregor
LonMcGregor
added
Reviewed
mervereis
added
the
Needs Review
2 participants
… features
Learners, PR Template
Self checklist
Completed all the tasks.
Task ID: CYF-1150