London | 26-SDC-March | Zobeir Rigi | Sprint 3 | implement-shell-tools#502
London | 26-SDC-March | Zobeir Rigi | Sprint 3 | implement-shell-tools#502Zobeir-Rigi wants to merge 14 commits into
Conversation
Zobeir-Rigi
added
the
Needs Review
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Can you try rewriting these using a package to manage the arguments? This can make it easier to handle options and command line access.
| continue; | ||
| } | ||
|
|
||
| console.log(file); |
There was a problem hiding this comment.
Are you correctly handling the -1 argument?
There was a problem hiding this comment.
I’ve fixed it. -1 is handled now. When it’s used, nothing changes since console.log already prints one per line.
LonMcGregor
added
Reviewed
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Think about why we have the -1 argument. What do we do if we want to change whether it prints on one line or over individual lines?
About WC, you now offer different argument combinations which is good. You might consider using a library/module to handle arguments as this makes it a lot easier.
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Great work. I hope it is clear why using something like commander in this case is helpful.
LonMcGregor
added
Complete
|
Closing PR because the SDC run has finished. Feel free to re-open if you're still working on it. |
3 participants
Self checklist
Shell tools implementation: cat, ls, and wc