London | 25-SDC-July | Andrei Filippov | Sprint 3 | Implement shell tools #126
Conversation
Droid-An
added
📅 Sprint 3
LonMcGregor
added
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Great work on these tasks, I have left some comments for improvements that could be made
| lines.pop(); | ||
| } | ||
|
|
||
| let lineNum = 1; |
There was a problem hiding this comment.
If I pass multiple files in, the line numbering resets to 1 for each file. Can you make your implementation work more like the original cat which uses a continuous numbering?
There was a problem hiding this comment.
Since I’m on macOS, the built-in cat also resets line numbers to 1 for each file. Its output looks like this:
cyf@cyfs-MacBook-Pro cat % cat -n sample-files/*
1 Once upon a time...
1 There was a house made of gingerbread.
1 It looked delicious.
2 I was tempted to take a bite of it.
3 But this seemed like a bad idea...
4
5 There's more to come, though...
implement-shell-tools/ls/ls.js
Outdated
| files.unshift("."); | ||
| return files; | ||
| } catch (err) { | ||
| console.error("Error reading directory:", err); |
There was a problem hiding this comment.
You have an error condition here if there was a problem reading the directory. Under what conditions can that error happen here?
(I have the same comment for the other flag)
There was a problem hiding this comment.
After reviewing the code, I realised this error condition will never occur. If we pass an invalid directory we get an error message before any flag is used, so I can remove that catch
implement-shell-tools/wc/wc.js
Outdated
| } | ||
| } | ||
| } | ||
| console.log([...output, path].join(" ")); |
There was a problem hiding this comment.
Can you think of a way to make the files align neatly, as if in a table, when multiple files of different word counts are present?
There was a problem hiding this comment.
yes, I can use padStart and length of the biggest value in a output as a length argument
LonMcGregor
added
Reviewed
Droid-An
added
Needs Review
|
Great. You're done with this task now! |
LonMcGregor
added
Complete
|
Awesome, thank you for the review |
2 participants
Learners, PR Template
Self checklist
Changelist
Done shell tools
Questions
Are my implementations alright?