-
-
Notifications
You must be signed in to change notification settings - Fork 42
London | 25-SDC-Nov | Aida Eslamimoghadam | Sprint 3 | Implement Shell Tools #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { program } from "commander"; | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think this import is ever used |
||
|
|
||
| program | ||
| .name("myCat") | ||
| .description("Simple file viewer") | ||
| .option("-n", "Number all output lines") | ||
| .option("-b", "Number non-blank output lines") | ||
| .argument("<path...>", "One or more file paths to show"); | ||
|
|
||
| program.parse(); | ||
|
|
||
| const files = program.args; | ||
| const opts = program.opts(); | ||
| let lineNumber = 1; | ||
|
|
||
| for (const filename of files) { | ||
| const content = await fs.readFile(filename, "utf-8"); | ||
|
|
||
| if (opts.n) { | ||
| const lines = content.split("\n"); | ||
|
|
||
| for (const line of lines) { | ||
| console.log(lineNumber + " " + line); | ||
| lineNumber++; | ||
| } | ||
| } else if (opts.b) { | ||
| const lines = content.split("\n"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be exported to the outer loop as this line repetition is unnecessary ( line 22 also) |
||
|
|
||
| for (const line of lines) { | ||
| if (line.trim() !== "") { | ||
| console.log(lineNumber + " " + line); | ||
| lineNumber++; | ||
| } | ||
| } | ||
| } else { | ||
| console.log(content); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { program } from "commander"; | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here the import is also never used |
||
|
|
||
| program | ||
| .name("myLs") | ||
| .description("my ls clone") | ||
| .option("-1", "one entry per line") | ||
| .option("-a", "show hidden files") | ||
| .argument("[paths...]", "file or directory paths"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good use of an optional parameter here! |
||
|
|
||
| program.parse(); | ||
|
|
||
| const opts = program.opts(); | ||
| let paths = program.args; | ||
|
|
||
| if (paths.length === 0) { | ||
| paths = ["."]; | ||
| } | ||
|
|
||
| for (const path of paths) { | ||
| const entries = await fs.readdir(path); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge fan of the name entreis here. Would one of your peers marking this know exactly what you are talkign about? Try to use a bit clearer naming patters :) |
||
|
|
||
| for (const file of entries) { | ||
| if (!opts.a && file.startsWith(".")) { | ||
| continue; | ||
| } | ||
|
|
||
| if (opts["1"]) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this function correctly? |
||
| console.log(file); | ||
| } else { | ||
| console.log(file + " "); | ||
| } | ||
| } | ||
|
|
||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "module", | ||
| "dependencies": { | ||
| "commander": "^14.0.2" | ||
| } | ||
| } |
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This solution works, but it is a bit non-standard compared to the original wc implementation. Notably, this utility is just about getting statistics, so you don't need to output the files content. That is what cat is for. Could you take another look at this one? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { program } from "commander"; | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also unnecessary import here. Can you have a google why? |
||
|
|
||
| program | ||
| .name("myLs") | ||
| .description("my ls clone") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this description is incorrect |
||
| .option("-l", "line count") | ||
| .option("-w", "words count") | ||
| .option("-c", "character count") | ||
| .option("-s", "character count without spaces") | ||
| .argument("[paths...]", "file or directory paths"); | ||
|
|
||
| program.parse(); | ||
|
|
||
| const opts = program.opts(); | ||
| let files = program.args; | ||
|
|
||
| if (files.length === 0) { | ||
| files = ["."]; | ||
| } | ||
|
|
||
| let totalLines = 0; | ||
| let totalWords = 0; | ||
|
|
||
| if (opts.l) { | ||
| for (const file of files) { | ||
| const content = await fs.readFile(file, "utf-8"); | ||
| console.log(content); | ||
| const lineCount = content.split("\n").length; | ||
|
|
||
| totalLines += lineCount; | ||
| } | ||
| console.log("Lines:", totalLines); | ||
| } | ||
| if (opts.w) { | ||
| for (const file of files) { | ||
| const content = await fs.readFile(file, "utf-8"); | ||
| console.log(content); | ||
|
|
||
| const wordCount = content.trim().split(/\s+/).length; | ||
| totalWords += wordCount; | ||
| } | ||
| console.log("Total words:", totalWords); | ||
| } | ||
| if (opts.c) { | ||
| let totalChars = 0; | ||
| for (const file of files) { | ||
| const content = await fs.readFile(file, "utf-8"); | ||
|
|
||
| totalChars += content.trim().length; | ||
| // const charList = content.trim().split(/\s+/); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remember we try to avoid comitting commented out code that is no longer needed |
||
| // for (const char of charList) { | ||
| // totalChars += char.length; | ||
| // } | ||
| } | ||
| console.log("Total characters:", totalChars); | ||
| } | ||
|
|
||
| if (opts.s) { | ||
| let totalCharsNoSpaces = 0; | ||
| for (const file of files) { | ||
| const content = await fs.readFile(file, "utf-8"); | ||
| const withoutSpaces = content.replace(/\s/g, ""); | ||
| totalCharsNoSpaces += withoutSpaces.length; | ||
| } | ||
| console.log("Total characters without spaces:", totalCharsNoSpaces); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good solution, but consider how the numbers will display if you had longer files. Is there some formatting you could add to improve it?