West Midlands | 26 March SDC | Iswat Bello | Sprint 3 | implement shell tools#454
Open
Iswanna wants to merge 7 commits intoCodeYourFuture:mainfrom
Open
West Midlands | 26 March SDC | Iswat Bello | Sprint 3 | implement shell tools#454Iswanna wants to merge 7 commits intoCodeYourFuture:mainfrom
Iswanna wants to merge 7 commits intoCodeYourFuture:mainfrom
Conversation
Add cat/cat.js which reads a file path from argv and prints the file contents using node fs promises and top-level await. Logs argv and path (debug output).
- Refactor to async readMultipleFiles() function - Support reading multiple files via Promise.all - Output concatenated file contents to stdout - Add error handling with exit code 1 on failure - Remove debug logging (argv, path) - Fix markdown list formatting in README (bullets)
- Replace manual argv parsing with commander.js for robust CLI handling - Add -n/--number flag to number all output lines - Add -b/--number-nonblank flag to number only non-empty lines - Refactor variable names for clarity (filePathsToRead, concatenatedContent, etc) - Improve error handling with process.exitCode = 1 - Remove debug logging
- Add ls/ls.js: reads a directory, supports -1 (one-per-line) and -a (include dotfiles) - Use commander for CLI flags, fs.promises.readdir for reading directories - Add error handling and set non-zero exit code on failure - docs(ls): normalize README bullets so example commands render correctly
- Add wc/wc.js: reads files and outputs line, word, and byte counts - Support multiple files with totals row - Use fs.promises.readFile for async file reading - Add error handling for missing files with non-zero exit code - Format output to match standard wc utility (padded columns) - docs(wc): normalize README bullets so example commands render correctly
- Migrate from manual argv parsing to commander.js for robust flag handling - Add support for -l (lines), -w (words), -c (bytes) flags - Refactor calculateFileStats to return object with descriptive properties - Rename printFormattedLine to printReport for clarity - Improve flag detection: default to all columns when no flags provided - Maintain totals row for multiple files - Update error messages to match standard wc format - Clean up JSDoc comments and code structure
- Rename parameters and variables for better descriptiveness: - files → filePaths - showAll → shouldShowAllStats - results → allFileStats - stats → fileStats - totals → grandTotals - label → displayName - s → stat (in reduce callbacks) - w → word (in filter callback) - output → outputColumns - format → formatColumn - printReport → printFormattedReport - Extract inline calculations to named variables in calculateFileStats() - lines, words, bytes for improved readability - Maintain all functionality and test coverage - No behavioral changes, purely refactoring for clarity
Iswanna
added
📅 Sprint 3
nedssoft
added
Review in progress
nedssoft
suggested changes
Apr 11, 2026
Comment on lines
+14
to
+16
| .action(async (filePaths, options) => { | ||
| const noFlagsProvided = !options.lines && !options.words && !options.bytes; | ||
| const showAll = noFlagsProvided; | ||
| const shouldShowAllStats = noFlagsProvided; |
There was a problem hiding this comment.
The variable noFlagsProvided appears a bit redundant. Do you think using shouldShowAllStats in line 15 wouldn't give the same clarity?
implement-shell-tools/wc/wc.js
Outdated
Comment on lines
61
to
65
| if (showAll || options.bytes) output.push(format(stats.byteCount)); | ||
| if (shouldShowAllStats || options.lines) outputColumns.push(formatColumn(stats.lineCount)); | ||
| if (shouldShowAllStats || options.words) outputColumns.push(formatColumn(stats.wordCount)); | ||
| if (shouldShowAllStats || options.bytes) outputColumns.push(formatColumn(stats.byteCount)); | ||
|
|
There was a problem hiding this comment.
Consider handling the shouldShowAllStats in one block to minimise the repetition. If the shouldShowAllStats condition kicks in, the program should not need to bother about other cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
This PR implements the
cat,ls, andwcUnix utilities in Node.js using ECMAScript Modules and Commander.js for CLI parsing.All implementations feature asynchronous file operations, robust error handling, and aligned output formatting.