Glasgow | 26- SDC-Mar | Taras Mykytiuk | Sprint 3 | Implement shell tools#447
Glasgow | 26- SDC-Mar | Taras Mykytiuk | Sprint 3 | Implement shell tools#447TarasMykytiuk wants to merge 3 commits into
Conversation
TarasMykytiuk
added
📅 Sprint 3
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
TarasMykytiuk
added
the
Needs Review
SlideGauge
added
Review in progress
SlideGauge
left a comment
SlideGauge
left a comment
There was a problem hiding this comment.
Could you address my comments please?
| async function displayFiles(paths, flags) { | ||
| let content = ''; | ||
| for (let i = 0; i < paths.length; i++) { | ||
| content += await fs.readFile(paths[i], "utf-8"); |
There was a problem hiding this comment.
What happens if we can't open one of the files?
| (flags.includes("-n") && !flags.includes("-b")) || | ||
| (flags.includes("-b") && lines[i] != "") | ||
| ) { | ||
| output = " " + (lineNumber).toString() + " " + lines[i]; |
There was a problem hiding this comment.
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
| (flags.includes("-n") && !flags.includes("-b")) || | ||
| (flags.includes("-b") && lines[i] != "") | ||
| ) { | ||
| output = " " + (lineNumber).toString() + " " + lines[i]; |
There was a problem hiding this comment.
Real cat uses tab separator between number and lines
| let output = lines[i]; | ||
| if ( | ||
| (flags.includes("-n") && !flags.includes("-b")) || | ||
| (flags.includes("-b") && lines[i] != "") |
There was a problem hiding this comment.
shouldn't we use strict inequality in javascript?
| 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.
What will happen if the file does not exist?
| let ending = ''; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| if (argv[i][0] == "-") { | ||
| flag = argv[i]; |
There was a problem hiding this comment.
What if we have multiple flags passed?
| let file = await fs.readFile(filePath, "utf-8"); | ||
| let linesNum = countLinesInString(file); | ||
| let wordsNum = countWordsInString(file); | ||
| let bytes = file.length; |
There was a problem hiding this comment.
what will happen with length if the file has non ASCII characters?
| let fileOutput = ''; | ||
| let filePath = dir + "/" + fileName; | ||
| let file = await fs.readFile(filePath, "utf-8"); | ||
| let linesNum = countLinesInString(file); |
There was a problem hiding this comment.
we do not reassign let- variables here, so we should use const instead
| } | ||
| } | ||
|
|
||
| function countLinesInString(str) { |
There was a problem hiding this comment.
like for splitting logic intot functions
| } | ||
|
|
||
| function countWordsInString(str) { | ||
| let words = str.replace(/\n/g, ' ').split(' '); |
There was a problem hiding this comment.
Do we need to take tabs and other whitespace into account?
2 participants
Learners, PR Template
Self checklist
Changelist
Implement shell tools exercises completed.