London | 26-SDC-Jul | Raihan Sharif | Sprint 3 | Implement shell tools

[RaihanSharif]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Changelist

I have implemented each of the shell tools for this assignment. I have checked output against the real shell tool using the sample inputs.

My put formatting isn't EXACTLY the same as the originals, but they contain all the current information in the correct order and almost the same format.

Task ID: CYF-1150

[SlideGauge]

Nice job, I got a couple of comments worth fixing, could you address them please?

[SlideGauge]

You declare let lineNum = 1 inside the for (const path of paths) loop. When you run cat -n sample-files/*.txt against the real cat, does the numbering restart at 1 for each file, or keep climbing across all of them? Where would lineNum need to be declared to match what you see?

[RaihanSharif]

cat -n sample-files/*.txt
     1  Once upon a time...
     1  There was a house made of gingerbread.
     1  It looked delicious.
     2  I was tempted to take a bite of it.
     3  But this seemed like a bad idea...
     4
     5  There's more to come, though...

node cat.js -n sample-files/*.txt
1 Once upon a time...
1 There was a house made of gingerbread.
1 It looked delicious.
2 I was tempted to take a bite of it.
3 But this seemed like a bad idea...
4 
5 There's more to come, though...

I don't see the difference. Other than the spacing at the start of each line.
I initally had the lineNum outside the loop so that the numbers would increment across files.
You can see this in the commit 5e36e1fccc18b08a5495f9ede132c9ed0b0cfa4b

[SlideGauge]

Your -n output on line 48 and -b output on line 56 both use ${lineNum} ${line} — number, one space, text. Put cat -n sample-files/1.txt next to the real one: how wide is the space before the 1, and what character sits between the number and the text? You noted formatting isn't exact — is this the gap you meant, and how close could padStart plus a tab get you?

[RaihanSharif]

cat -n sample-files/*.txt        
     1  Once upon a time...
     1  There was a house made of gingerbread.
     1  It looked delicious.
     2  I was tempted to take a bite of it.
     3  But this seemed like a bad idea...
     4
     5  There's more to come, though...
cat % node cat.js -n sample-files/*.txt
     1 Once upon a time...
     1 There was a house made of gingerbread.
     1 It looked delicious.
     2 I was tempted to take a bite of it.
     3 But this seemed like a bad idea...
     4 
     5 There's more to come, though...

It appears to be 5 spaces. I have added this change.

I was wrong, I will fix it again. It appears to be a padded, right justified number with total length of 6, and then tab before printing the line.

I have fixed by adding this:
console.log(`${String(lineNum).padStart(6, " ")}\t${line}`);

[SlideGauge]

Compare node ls.js -1 -a sample-files to the real ls -1 -a sample-files, line by line. Which two entries does the real one show that yours doesn't? Does the fs.readdirSync call on line 31 ever return those, and if not, where could they come from?

[RaihanSharif]

You mean this?

. 
. .

Those are current and parent directories. I figured it'd be ok to leave them out as they're not captured by process.

I've added them now.

function getPathEntries(path, aFlag = flags.has("a")) {
    let entries = fs.readdirSync(path);
    entries = [".", "..", ...entries];

    if (!aFlag) {
        entries = entries.filter((e) => !e.startsWith("."));
    }
    return entries;
}

Appears to be working as expected.

[SlideGauge]

Your output matches here, but is the order fs.readdirSync (line 31) returns entries in actually guaranteed? What does real ls do to its list before printing — and would yours still match on a machine that read the directory in a different order?

[RaihanSharif]

No it's not guaranteed. I made an assumption given the entries appears to be in the right order each time.

Real ls orders the path strings alphabetically before output.

I have now added a line to sort the entries.

function getPathEntries(path, aFlag = flags.has("a")) {
    let entries = fs.readdirSync(path);
    entries = [".", "..", ...entries];
    entries.sort();  // sort before output.
    if (!aFlag) {
        entries = entries.filter((e) => !e.startsWith("."));
    }
    return entries;
}

[SlideGauge]

Your "Is a directory" message on line 33 goes through console.log. Which stream do the real shell tools send error messages to - stdout or stderr - and does it matter if someone pipes your output into another command? (Not exercised by the sample files, just worth knowing.)

[RaihanSharif]

Thank you for pointing this out. I have changed it now to output errors to stderr using console.error

[SlideGauge]

What is chownSync (imported on line 2) used for in this file? And on line 58, what does the bare statement file.size; do, given file is a string? Would the program behave any differently if both were deleted?

[RaihanSharif]

chownSync must have snuck in because of vs code auto imort. I don't remember putting it in there.

fs.statSync(path).size;

This is the size of the file in bytes. Doesn't matter what type of file it is.

from man wc:
-c The number of bytes in each input file is written to the standard output.

if I had done something like:

const file = fs.readFileSync(path, "utf-8");
const byteCount = file.length;

Then the byteCount would actually be a character count.

[SlideGauge]

Nice job! Review complete!