London | 26-SDC-Mar | Beko | Sprint 3 | Implement Shell Tools#500
London | 26-SDC-Mar | Beko | Sprint 3 | Implement Shell Tools#500Abubakar-Meigag wants to merge 1 commit into
Conversation
Abubakar-Meigag
added
📅 Sprint 3
SlideGauge
left a comment
SlideGauge
left a comment
There was a problem hiding this comment.
I've got several notes from my side, could you fix them please? Otherwise good job
| if (lines[lines.length - 1] === '') lines.pop(); | ||
|
|
||
| for (const line of lines) { | ||
| const isBlank = line.trim() === ''; |
There was a problem hiding this comment.
real cat treats lines with spaces as real lines and does not ignore them
| content = await fs.readFile(filePath, 'utf8'); | ||
| } catch { | ||
| console.error(`cat: ${filePath}: No file or directory exists`); | ||
| process.exit(1); |
There was a problem hiding this comment.
This will stop processing of other files. Real cat continues processing of other files.
|
|
||
| const targetDir = paths[0] ?? '.'; | ||
|
|
||
| const entries = await fs.readdir(targetDir); |
| const result = showAll ? ['.', '..', ...entries] : entries.filter(e => !e.startsWith('.')); | ||
|
|
||
| for (const entry of result) { | ||
| console.log(entry); |
There was a problem hiding this comment.
Real ls default output is space-separated on one line; one-per-line requires the -1 flag. This always uses one-per-line regardless. The -1 flag needs to be implemented and the default output should join entries with spaces (plus a trailing newline).
|
|
||
| const targetDir = paths[0] ?? '.'; | ||
|
|
||
| const entries = await fs.readdir(targetDir); |
There was a problem hiding this comment.
entries is not sorted. readdir order is filesystem-dependent. Sort before output: entries.sort(). The . and .. entries should remain first.
|
|
||
| const showAll = dash.includes('-a'); | ||
|
|
||
| const targetDir = paths[0] ?? '.'; |
| totalWords += words; | ||
| totalBytes += bytes; | ||
|
|
||
| if (forLines) { |
There was a problem hiding this comment.
What happens if we have several flags simultaneously?
| content = await fs.readFile(filePath, 'utf8'); | ||
| } catch { | ||
| console.error(`wc: ${filePath}: No file or directory exists`); | ||
| process.exit(1); |
There was a problem hiding this comment.
Same situation: if one file is problematic, the whole execution stops. Real wc continues processing of other files.
| } | ||
|
|
||
| if (filePaths.length > 1) { | ||
| if (forLines) { |
There was a problem hiding this comment.
What if we have several flags passed?
SlideGauge
added
Reviewed
2 participants
Self checklist
Complete shell tools for sprint 3