-
-
Notifications
You must be signed in to change notification settings - Fork 88
London | 26-SDC-Mar | Mariia Serhiienko | Sprint 3 | Implement shell tools #499
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
b40010c
19cae55
ccf9aa6
f3923c8
5c84e31
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,56 @@ | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
|
||
| const args = process.argv.slice(2); | ||
|
|
||
| if (args.length === 0) { | ||
| console.error("Usage: node cat.js [-n] [-b] <file...>"); | ||
| process.exit(1); | ||
| } | ||
| const showAllLineNumbers = args.includes("-n"); | ||
| const showNonBlankNumbers = args.includes("-b"); | ||
| const supportedFlags = ["-n", "-b"]; | ||
| const unknownFlags = args.filter( | ||
| (arg) => arg.startsWith("-") && !supportedFlags.includes(arg), | ||
| ); | ||
|
|
||
| if (unknownFlags.length > 0) { | ||
| console.error(`cat: invalid option -- '${unknownFlags[0].slice(1)}'`); | ||
|
Member
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. Optional: It's often useful to tell people about all of the problems you find, if it's easy to do so, so they can fix all of them - the experience of running: % cat -f -x /tmp/t
cat: invalid option -- f
% cat -x /tmp/t
cat: invalid option: -- x
% cat /tmp/t
It worked!is quite frustrating compared to: % cat -f -x /tmp/t
cat: invalid options: f, x
% cat /tmp/t
It worked! |
||
| process.exit(1); | ||
| } | ||
|
|
||
| const filePaths = args.filter((arg) => !arg.startsWith("-")); | ||
|
|
||
| let lineNumber = 1; | ||
|
|
||
| for (const filePath of filePaths) { | ||
| const content = await fs.readFile(filePath, "utf-8"); | ||
|
|
||
| if (!showAllLineNumbers && !showNonBlankNumbers) { | ||
|
Member
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. These are really clear variable names, which makes this if statement read very clearly - well done!
Author
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. Thank you! |
||
| process.stdout.write(content); | ||
| } else { | ||
| const lines = content.split("\n"); | ||
|
|
||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i]; | ||
|
|
||
| const isLastLine = i === lines.length - 1; | ||
|
|
||
| if (isLastLine && line === "") { | ||
| break; | ||
| } | ||
|
|
||
| const isBlankLine = line.trim() === ""; | ||
| const needsLineNumber = | ||
| showAllLineNumbers || (showNonBlankNumbers && !isBlankLine); | ||
|
|
||
| if (needsLineNumber) { | ||
| const paddedNumber = String(lineNumber).padStart(6, " "); | ||
| process.stdout.write(`${paddedNumber}\t${line}\n`); | ||
| lineNumber++; | ||
| } else if (showNonBlankNumbers && isBlankLine) { | ||
| process.stdout.write("\n"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
|
||
| const args = process.argv.slice(2); | ||
|
|
||
| const showHidden = args.includes("-a"); | ||
|
|
||
| const supportedFlags = ["-1", "-a"]; | ||
| const unknownFlags = args.filter( | ||
| (arg) => arg.startsWith("-") && !supportedFlags.includes(arg), | ||
| ); | ||
|
|
||
| if (unknownFlags.length > 0) { | ||
| console.error(`ls: invalid option -- '${unknownFlags[0].slice(1)}'`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const paths = args.filter((arg) => !arg.startsWith("-")); | ||
|
|
||
| const targetPaths = paths.length > 0 ? paths : [process.cwd()]; | ||
|
|
||
| for (const targetPath of targetPaths) { | ||
| if (targetPaths.length > 1) { | ||
| process.stdout.write(`${targetPath}:\n`); | ||
| } | ||
|
|
||
| let entries; | ||
|
|
||
| try { | ||
| entries = await fs.readdir(targetPath); | ||
| } catch (error) { | ||
| console.error( | ||
|
Member
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. Currently if there's an error (e.g. if you specify a path that doesn't exist), the exit code of this program will be 0. Is that what we want? |
||
| `ls: cannot access '${targetPath}': No such file or directory`, | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| if (!showHidden) { | ||
| entries = entries.filter((entry) => !entry.startsWith(".")); | ||
| } | ||
|
|
||
| if (showHidden) { | ||
| entries = [".", "..", ...entries]; | ||
| } | ||
|
|
||
| entries.sort((a, b) => { | ||
| const cleanA = a.replace(/^\.+/, ""); | ||
| const cleanB = b.replace(/^\.+/, ""); | ||
| return cleanA.localeCompare(cleanB); | ||
| }); | ||
|
|
||
| for (const entry of entries) { | ||
| process.stdout.write(entry + "\n"); | ||
| } | ||
|
|
||
| if (targetPaths.length > 1) { | ||
| process.stdout.write("\n"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "type": "module" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { promises as fs } from "node:fs"; | ||
| import process from "node:process"; | ||
|
|
||
| const args = process.argv.slice(2); | ||
|
|
||
| const expandedArgs = []; | ||
| for (const arg of args) { | ||
| if (arg.startsWith("-") && arg.length > 2) { | ||
| for (const char of arg.slice(1)) { | ||
| expandedArgs.push(`-${char}`); | ||
| } | ||
| } else { | ||
| expandedArgs.push(arg); | ||
| } | ||
| } | ||
|
|
||
| const showLines = expandedArgs.includes("-l"); | ||
| const showWords = expandedArgs.includes("-w"); | ||
| const showBytes = expandedArgs.includes("-c"); | ||
|
|
||
| const supportedFlags = ["-l", "-w", "-c"]; | ||
| const unknownFlags = expandedArgs.filter( | ||
| (arg) => arg.startsWith("-") && !supportedFlags.includes(arg), | ||
| ); | ||
|
|
||
| if (unknownFlags.length > 0) { | ||
| console.error(`wc: invalid option -- '${unknownFlags[0].slice(1)}'`); | ||
| process.exit(1); | ||
| } | ||
|
Comment on lines
+6
to
+29
Member
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 is good code, and works well considering different edge cases (well done!), but is a lot of code you've had to write to handle this :) you don't need to for this exercise, but you may want to look at a library like https://github.com/tj/commander.js which can do a lot of this for you. |
||
|
|
||
| const noSpecificFlag = !showLines && !showWords && !showBytes; | ||
|
|
||
| const filePaths = expandedArgs.filter((arg) => !arg.startsWith("-")); | ||
|
|
||
| if (filePaths.length === 0) { | ||
| console.error("Usage: node wc.js [-l] [-w] [-c] <file...>"); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const results = []; | ||
|
|
||
| for (const filePath of filePaths) { | ||
| const content = await fs.readFile(filePath, "utf-8"); | ||
|
|
||
| const lines = content.endsWith("\n") | ||
| ? content.split("\n").length - 1 | ||
| : content.split("\n").length; | ||
|
|
||
| const words = content.split(/\s+/).filter((w) => w.length > 0).length; | ||
|
|
||
| const bytes = Buffer.byteLength(content, "utf-8"); | ||
|
|
||
| results.push({ filePath, lines, words, bytes }); | ||
| } | ||
|
|
||
| const totalLines = results.reduce((sum, r) => sum + r.lines, 0); | ||
| const totalWords = results.reduce((sum, r) => sum + r.words, 0); | ||
| const totalBytes = results.reduce((sum, r) => sum + r.bytes, 0); | ||
|
|
||
| function getCounts(lines, words, bytes) { | ||
| const counts = []; | ||
| if (noSpecificFlag || showLines) counts.push(lines); | ||
| if (noSpecificFlag || showWords) counts.push(words); | ||
| if (noSpecificFlag || showBytes) counts.push(bytes); | ||
|
Comment on lines
+62
to
+64
Member
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. These work, but quite tightly couple the CLI flags to the behaviour here. We don't need to use only what flags we happened to expose to users when writing our logic - we can transform things ourselves. Can you think how you could set up if (showLines) counts.push(lines);
if (showWords) counts.push(words);
if (showBytes) counts.push(bytes);but still work correctly when not flags were passed? |
||
| return counts; | ||
| } | ||
|
|
||
| const maxNumber = Math.max(...getCounts(totalLines, totalWords, totalBytes)); | ||
|
|
||
| const width = String(maxNumber).length + 1; | ||
|
|
||
| function formatLine(counts, label) { | ||
| const parts = counts.map((n) => String(n).padStart(width, " ")); | ||
| return parts.join("") + " " + label; | ||
| } | ||
|
|
||
| for (const { filePath, lines, words, bytes } of results) { | ||
| const counts = getCounts(lines, words, bytes); | ||
| process.stdout.write(formatLine(counts, filePath) + "\n"); | ||
| } | ||
|
|
||
| if (results.length > 1) { | ||
| const totals = getCounts(totalLines, totalWords, totalBytes); | ||
| process.stdout.write(formatLine(totals, "total") + "\n"); | ||
| } | ||
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.
Imagine if I called this programme with a different flag, e.g.
node cat.js -x /some/file. I probably passed that-xflag expecting it would do something. But your programme just ignores it.What do you think we should do if we run into a flag our programme doesn't understand?
This applies to all three tools.
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.
Thanks for pointing this out! You're right, silently ignoring unknown flags is confusing for the user. I've added a check in all three tools that shows an error message and exits if the user passes a flag we don't recognise. For example, node cat.js -x file.txt will now print cat: invalid option -- 'x' instead of just running as if nothing happened.