London SDC | Samira Hekmati | Module-Tools | Implement-Shell-Tools in JS | Week 3 #53

samirahekmati · 2025-03-30T15:30:03Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

blorente

Thank you for your PR! I'm Borja, a volunteer, and I'll be reviewing your code.

The implementations of cat, ls, and wc are very good! I have left a few edge cases that are not covered, as well as a few suggestions, but you're definitely on the right track :)

I've also really enjoyed the comments you've left yourself. Great job understanding what all the functions do!

Besides that, my only comment is that when running the tools, I needed to write node wc.js wc <arguments>. I would have expected to be able to write node wc.js <arguments> (not an extra wc). Is that an intentional choice?

Keep up the good work!

blorente · 2025-05-15T15:58:48Z

implement-shell-tools/cat/cat.js

+
+        if (options.number) {
+          lines = lines.map((line) => `${lineCounter++} ${line}`);
+        } else if (options.nonBlank) {


There seems to be something wrong with the output. When I run this code, the lines on multiple files are counted the with the same counter:

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

However, when we run cat, we can see that every line number is counted individually:

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

How can we modify the code so that the lines are counted correctly?

blorente · 2025-05-15T15:59:09Z

implement-shell-tools/cat/cat.js

+    let lineCounter = 1; // Continuous line number for `-n`
+    let nonBlankCounter = 1; // Non-blank line number for `-b`


Comments can get out of date. Is there a way you can codify this intention in the code? e.g. via variable name.

blorente · 2025-05-15T16:00:51Z

implement-shell-tools/cat/package.json

+  "main": "cat.js",
+  "type": "module",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"


Do we need this script? For a stretch goal, is there a script you could write to run cat with npm run cat?

blorente · 2025-05-15T16:03:06Z

implement-shell-tools/ls/ls.js

+//note to myself:
+// path.resolve() converts a relative path into an absolute path. 
+//The last argument (dir) is resolved relative to the first argument.
+//syntax: path.resolve(...paths);
+//process.cwd() Returns the current working directory of the process (i.e., where the user ran the command from).It does NOT return the script’s directory.
+//targetDir is created to store this resolved absolute path for reliable use in fs.readdirSync().


These notes are great! In a real code submission I'd ask you to remove them, but as this is a learning exercise, well done getting to the bottom of this!

The same applies with the rest of the comments. In general, it's safe to assume that the person reading the code has easy access to the documentation, so you don't need to explain, for instance, what the arguments to readdir do. In this case, kudos for makign the effort to understand them!

blorente · 2025-05-15T16:14:43Z

implement-shell-tools/ls/ls.js

+program
+    .argument('[directory]', 'Directory to list', '.')//This specifies the directory argument. If no directory is provided, it defaults to the current directory (.)
+    .option("-l, --format", "List files in a single column") // Add -1 flag
+    .option("-a, --hidden", "Show hidden files (like ls -a)") // Add -a flag


When -a is set, it should also print the current directory (.) and the parent diractory (..).

➜ ls -1 -a sample-files . .. .hidden.txt 1.txt 2.txt 3.txt dir

How could we modify this solution so that we do print them?

blorente · 2025-05-15T16:19:36Z

implement-shell-tools/ls/ls.js

+            if(options.format){
+                console.log(filteredFiles.join("\n"))// Print files in a single column (default `ls -1` behavior)
+            }else{
+                console.log(filteredFiles.join(" "))// Print files space-separated (default `ls` behavior)


In ls, files are printed with a certain amount of padding, so that if we have many files, they appear to be in a table:

➜ /bin/ls ls.js package-lock.json README.md node_modules package.json sample-files

This is not part of the assignment brief but, as a streacth goal, would you like to consider how to pad the outputs like that?

blorente · 2025-05-15T16:47:50Z

implement-shell-tools/ls/ls.js

+    .option("-l, --format", "List files in a single column") // Add -1 flag
+    .option("-a, --hidden", "Show hidden files (like ls -a)") // Add -a flag
+    .action(async (directory) => {
+        const resolvedPath = path.resolve(directory);


What happens when we receive more than one argument? (e.g. node ls.js directory1 directory2)

blorente · 2025-05-15T17:36:09Z

implement-shell-tools/wc/wc.js

+program
+  .command("wc <files...>") //The <files...> syntax means it accepts one or more file names as arguments.
+  .description("Count lines, words, and bytes in text files")
+  .option("-l, --lines", "Count lines")


In wc when counting stats from multiple files, there is a total line at the end:

➜ wc -l sample-files/* 1 sample-files/1.txt 1 sample-files/2.txt 5 sample-files/3.txt 7 total

How could we implement that functionality into this program?

blorente · 2025-05-15T17:40:44Z

implement-shell-tools/wc/wc.js

+  .description("Count lines, words, and bytes in text files")
+  .option("-l, --lines", "Count lines")
+  .option("-w, --words", "Count words")
+  .option("-c, --bytes", "Count bytes")


In your program, numbers are left-aligned:

➜ node wc.js wc -c sample-files/* sample-files/1.txt:20 sample-files/2.txt:39 sample-files/3.txt:125

However, in wc they are padded so that large numbers and small numbers appear neatly on a table:

➜ wc -c sample-files/* 20 sample-files/1.txt 39 sample-files/2.txt 125 sample-files/3.txt 184 total

Note how it's easy to tell that 35 has one digit less than 125.
As a stretch goal, would you like to implement some padding so that large numbers are displayed neatly?

blorente · 2025-05-15T17:41:56Z

implement-shell-tools/wc/wc.js

+program
+  .command("wc <files...>") //The <files...> syntax means it accepts one or more file names as arguments.
+  .description("Count lines, words, and bytes in text files")
+  .option("-l, --lines", "Count lines")


When counting lines, your program counts one more than wc:

➜ wc -l sample-files/3.txt 5 sample-files/3.txt ➜ node wc.js wc -l sample-files/3.txt sample-files/3.txt: 6

Why does this happen? How could we fix it so that we print the right number of lines?

samirahekmati added 6 commits March 18, 2025 16:14

write a simple script that reads file(no flag yet)

25a320a

Installed Commander.js for command-line parsing

29aa174

Implement 'cat' command with line numbering

6eff5a5

implement ls command with -a and -1 options

95f3037

clean code

6221ea4

Implement wc for counting lines, words & bytes

bff17f7

samirahekmati added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Participant to add when requesting review labels Mar 30, 2025

blorente reviewed May 15, 2025

View reviewed changes

blorente added 👀 Review Requirements Changes requested to meet requirements Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review labels May 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

London SDC | Samira Hekmati | Module-Tools | Implement-Shell-Tools in JS | Week 3 #53

London SDC | Samira Hekmati | Module-Tools | Implement-Shell-Tools in JS | Week 3 #53

Uh oh!

samirahekmati commented Mar 30, 2025 •

edited

Loading

Uh oh!

blorente left a comment

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

blorente May 15, 2025

Uh oh!

Uh oh!

		let lineCounter = 1; // Continuous line number for `-n`
		let nonBlankCounter = 1; // Non-blank line number for `-b`

Uh oh!

London SDC | Samira Hekmati | Module-Tools | Implement-Shell-Tools in JS | Week 3 #53

Are you sure you want to change the base?

London SDC | Samira Hekmati | Module-Tools | Implement-Shell-Tools in JS | Week 3 #53

Uh oh!

Conversation

samirahekmati commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Questions

Uh oh!

blorente left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samirahekmati commented Mar 30, 2025 •

edited

Loading