-
-
Notifications
You must be signed in to change notification settings - Fork 105
Glasgow | 26- SDC-Mar | Taras Mykytiuk | Sprint 3 | Implement shell tools #447
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
Changes from 3 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 process from "node:process"; | ||
| import { promises as fs } from "node:fs"; | ||
|
|
||
| const argv = process.argv.slice(2, process.argv.length); | ||
| const flags = []; | ||
| const paths = []; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| if (argv[i][0] == "-") { | ||
| flags.push(argv[i]); | ||
| } else { | ||
| paths.push(argv[i]); | ||
| } | ||
| } | ||
|
|
||
| displayFiles(paths, flags); | ||
|
|
||
| async function displayFiles(paths, flags) { | ||
| let content = ''; | ||
| for (let i = 0; i < paths.length; i++) { | ||
| content += await fs.readFile(paths[i], "utf-8"); | ||
| } | ||
|
|
||
| const lines = content.split("\n"); | ||
| if (lines[lines.length - 1] == '') { | ||
| lines.pop(); | ||
| } | ||
| let lineNumber = 1; | ||
| for (let i = 0; i < lines.length; i++) { | ||
| let output = lines[i]; | ||
| if ( | ||
| (flags.includes("-n") && !flags.includes("-b")) || | ||
| (flags.includes("-b") && lines[i] != "") | ||
|
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. shouldn't we use strict inequality in javascript?
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. Fixed |
||
| ) { | ||
| output = " " + (lineNumber).toString() + " " + lines[i]; | ||
|
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 we always prepend 5 spaces. But we need to right-align into a fixed-width field. (So numbers like 10, 100 etc does not shift the column
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. Added padStart 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. Real cat uses tab separator between number and lines
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. Added separator. |
||
| lineNumber++; | ||
| } | ||
| console.log(output); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import process from "node:process"; | ||
| import { promises as fs } from "node:fs"; | ||
|
|
||
| const argv = process.argv.slice(2, process.argv.length); | ||
| const currentDir = './'; | ||
| const flags = []; | ||
| let path = ''; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| if (argv[i][0] == "-") { | ||
| flags.push(argv[i]); | ||
| } else { | ||
| path = argv[i]; | ||
| } | ||
| } | ||
| if (path == '') path = currentDir; | ||
|
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. Do we have multiple path support? |
||
|
|
||
| const content = 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. Raw reddir includes hidden files, is it intentional?
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. Fxed |
||
|
|
||
| if (flags.includes("-l")) { | ||
|
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. We should use flag -1, not -l
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. Fixed |
||
| for (let i = 0; i < content.length; i++) { | ||
| let line = content[i]; | ||
| if (!flags.includes("-a") && line[0] == ".") continue; | ||
| console.log(line); | ||
| } | ||
| } else { | ||
| console.log(content.join(" ")); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| { | ||
| "name": "implement-shell-tools", | ||
| "version": "1.0.0", | ||
| "type": "module", | ||
| "description": "Your task is to re-implement shell tools you have used.", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| }, | ||
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import process from "node:process"; | ||
| import { promises as fs } from "node:fs"; | ||
|
|
||
| const argv = process.argv.slice(2, process.argv.length); | ||
| let flag = ''; | ||
| const paths = []; | ||
| let dir = ''; | ||
| let ending = ''; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| if (argv[i][0] == "-") { | ||
| flag = argv[i]; | ||
|
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. What if we have multiple flags passed?
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. Changed flag to flags array. |
||
| } else { | ||
| paths.push(argv[i]); | ||
| } | ||
| } | ||
|
|
||
| if (paths.length > 0) { | ||
| dir = paths[0].slice(0, paths[0].indexOf("/")); | ||
|
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 approach means node wc.js dir/a.txt dir/b.txt reads every file in dir/ instead of just a.txt and b.txt. |
||
| ending = paths[0].slice(paths[0].indexOf("/") + 1); | ||
| }; | ||
|
|
||
| const files = await resolveQuery(paths, dir, ending); | ||
| await logFilesInfo(files, flag); | ||
|
|
||
| async function resolveQuery(paths, dir, ending) { | ||
| let files = []; | ||
| if (paths.length > 1) { | ||
| files = await fs.readdir(dir); | ||
| } else { | ||
| files = [ending]; | ||
| } | ||
| return files; | ||
| } | ||
|
|
||
| async function logFilesInfo(files, flag) { | ||
| let totalLines = 0; | ||
| let totalWords = 0; | ||
| let totalBytes = 0; | ||
| let totalOutput = ''; | ||
| for (const fileName of files) { | ||
| let fileOutput = ''; | ||
| let filePath = dir + "/" + fileName; | ||
| let file = await fs.readFile(filePath, "utf-8"); | ||
|
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. What will happen if the file does not exist?
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. Added try-catch |
||
| let linesNum = countLinesInString(file); | ||
|
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. we do not reassign let- variables here, so we should use const instead
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. Added const where it possible. |
||
| let wordsNum = countWordsInString(file); | ||
| let bytes = file.length; | ||
|
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. what will happen with length if the file has non ASCII characters?
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. Buffer.byteLength now used |
||
| totalLines += linesNum; | ||
| totalWords += wordsNum; | ||
| totalBytes += bytes; | ||
| if (flag == "-l") { | ||
| fileOutput = linesNum + " " + filePath; | ||
| } else if (flag == "-w") { | ||
| fileOutput = wordsNum + " " + filePath; | ||
| } else if (flag == '-c') { | ||
| fileOutput = bytes + " " + filePath; | ||
| } else { | ||
| fileOutput = linesNum + " " + wordsNum + " " + bytes + " " + filePath; | ||
| } | ||
| console.log(fileOutput); | ||
| } | ||
| if (files.length > 1) { | ||
| if (flag == "-l") { | ||
| totalOutput = totalLines + " total"; | ||
| } else if (flag == "-w") { | ||
| totalOutput = totalWords + " total"; | ||
| } else if (flag == '-c') { | ||
| totalOutput = totalBytes + " total"; | ||
| } else { | ||
| totalOutput = totalLines + " " + totalWords + " " + totalBytes + " total"; | ||
| } | ||
| console.log(totalOutput); | ||
| } | ||
| } | ||
|
|
||
| function countLinesInString(str) { | ||
|
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. like for splitting logic intot functions
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. Thanks. |
||
| let lines = str.split('\n'); | ||
| let linesNum = lines.length; | ||
| if (lines[lines.length - 1] == '') { | ||
| linesNum--; | ||
| } | ||
| return linesNum; | ||
| } | ||
|
|
||
| function countWordsInString(str) { | ||
| let words = str.replace(/\n/g, ' ').split(' '); | ||
|
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. Do we need to take tabs and other whitespace into account?
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. Refactored. |
||
| words = words.filter((word) => { | ||
| return (word != '') | ||
| }); | ||
| let wordsNum = words.length; | ||
| return wordsNum; | ||
| } | ||
|
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. Trailing new line is missing
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. Fixed |
||
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.
What happens if we can't open one of the files?
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.
Added try-catch.