-
Notifications
You must be signed in to change notification settings - Fork 2
Mirror of 408 #5
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,60 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| function cat(files, options) { | ||
| let lineNumber = 1; | ||
|
|
||
| files.forEach((file) => { | ||
| const filePath = path.resolve(file); | ||
|
|
||
| try { | ||
| const data = fs.readFileSync(filePath, 'utf8'); | ||
| const lines = data.split('\n'); | ||
|
|
||
| lines.forEach((line) => { | ||
| if (options.numberNonEmpty && line.trim()) { | ||
| console.log(`${lineNumber}\t${line}`); | ||
| lineNumber++; | ||
| } else if (options.numberLines) { | ||
| console.log(`${lineNumber}\t${line}`); | ||
| lineNumber++; | ||
|
Comment on lines
+17
to
+22
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've noticed that the logic for printing lines with or without numbers is repeated in two branches of your if-else statement (lines 17-22). If you ever wanted to change how lines are printed, you'd have to update it in both places. How could you extract this logic into a helper function to avoid repeating yourself and make future changes easier?
Member
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. Great comment! |
||
| } else { | ||
| console.log(line); | ||
| } | ||
| }); | ||
| } catch (err) { | ||
| console.error(`cat: ${file}: No such file or directory`); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| function main() { | ||
| const args = process.argv.slice(2); | ||
| const options = { | ||
| numberLines: false, | ||
| numberNonEmpty: false, | ||
|
Comment on lines
+36
to
+37
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. The option names 'numberLines' and 'numberNonEmpty' are a bit similar and could be confusing. Could you think of names that make it clearer what each option does, especially for someone reading the code for the first time?
Member
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. Borderline useful comment, though I'd maybe have phrased this as "so that if someone sees either one option name without seeing the other, they would know the difference" |
||
| }; | ||
|
|
||
| const files = []; | ||
|
|
||
| args.forEach((arg) => { | ||
| if (arg === '-n') { | ||
| options.numberLines = true; | ||
| } else if (arg === '-b') { | ||
| options.numberNonEmpty = true; | ||
| } else { | ||
| files.push(arg); | ||
| } | ||
| }); | ||
|
|
||
| if (files.length === 0) { | ||
| console.error('Usage: node cat.js [-n | -b] <file>...'); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| cat(files, options); | ||
| } | ||
|
|
||
| main(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| function listFiles(directory, options) { | ||
| try { | ||
| const files = fs.readdirSync(directory, { withFileTypes: true }); | ||
|
|
||
| files.forEach((file) => { | ||
| if (!options.all && file.name.startsWith('.')) { | ||
| return; // Skip hidden files unless -a is specified | ||
| } | ||
| console.log(file.name); | ||
| }); | ||
| } catch (err) { | ||
| console.error(`ls: cannot access '${directory}': No such file or directory`); | ||
| } | ||
| } | ||
|
|
||
| function main() { | ||
| const args = process.argv.slice(2); | ||
| const options = { | ||
| all: false, | ||
| }; | ||
|
|
||
| let directories = ['.']; | ||
|
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. You use a variable called 'directories', but in your code, it always contains only one directory (or the default '.'). Do you think the plural name might mislead someone into thinking it can handle multiple directories at once? What name might better reflect its actual use? |
||
|
|
||
| args.forEach((arg) => { | ||
| if (arg === '-1') { | ||
| // -1 is the default behavior, so no action needed | ||
| } else if (arg === '-a') { | ||
| options.all = true; | ||
| } else { | ||
| directories = [arg]; | ||
|
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. You use a variable called 'directories', but in your code, it always contains only one directory (or the default '.'). Do you think the plural name might mislead someone into thinking it can handle multiple directories at once? What name might better reflect its actual use?
Member
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. Ideally this comment wouldn't be duplicated |
||
| } | ||
| }); | ||
|
|
||
| directories.forEach((directory) => { | ||
| listFiles(directory, options); | ||
| }); | ||
| } | ||
|
|
||
| main(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| function countFile(filePath, options) { | ||
| try { | ||
| const data = fs.readFileSync(filePath, 'utf8'); | ||
|
|
||
| const lines = data.split('\n').length; | ||
| const words = data.split(/\s+/).filter(Boolean).length; | ||
| const bytes = Buffer.byteLength(data, 'utf8'); | ||
|
|
||
| if (options.lines) { | ||
| console.log(`${lines}\t${filePath}`); | ||
| } else if (options.words) { | ||
| console.log(`${words}\t${filePath}`); | ||
| } else if (options.bytes) { | ||
| console.log(`${bytes}\t${filePath}`); | ||
| } else { | ||
| console.log(`${lines}\t${words}\t${bytes}\t${filePath}`); | ||
| } | ||
| } catch (err) { | ||
| console.error(`wc: ${filePath}: No such file or directory`); | ||
| } | ||
| } | ||
|
|
||
| function main() { | ||
| const args = process.argv.slice(2); | ||
| const options = { | ||
| lines: false, | ||
| words: false, | ||
| bytes: false, | ||
| }; | ||
|
|
||
| const files = []; | ||
|
|
||
| args.forEach((arg) => { | ||
| if (arg === '-l') { | ||
| options.lines = true; | ||
| } else if (arg === '-w') { | ||
| options.words = true; | ||
| } else if (arg === '-c') { | ||
| options.bytes = true; | ||
| } else { | ||
| files.push(arg); | ||
| } | ||
| }); | ||
|
|
||
| if (files.length === 0) { | ||
| console.error('Usage: wc [-l | -w | -c] <file>...'); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| files.forEach((file) => { | ||
| const filePath = path.resolve(file); | ||
| countFile(filePath, options); | ||
| }); | ||
| } | ||
|
|
||
| main(); |
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.
The variable 'lineNumber' is declared outside the loop that goes through each file. This means if you process multiple files, the line numbers will continue from one file to the next, rather than restarting for each file. Is this the behavior you want? If not, where could you declare 'lineNumber' so that it resets for each file?
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.
Not sure about this - see #4 (comment)