Skip to content

London| Elhadj Abdoul Diallo| Module-Tools | WEEK3 - Implement-shell-tools #25

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

Open
wants to merge 64 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
f5382f7
Add implementation for cat shell tool
Mar 3, 2025
cfa60fe
Refactor cat implementation to support reading multiple files asynchr…
Mar 3, 2025
48355e1
Implement display function to output contents of the files
Mar 3, 2025
16944c0
Fix variable name for file paths in cat implementation
Mar 3, 2025
677386b
Add package.json and package-lock.json for dependency management
Mar 3, 2025
2fc1242
Enhance cat implementation with command-line options for line numberi…
Mar 3, 2025
6c5e0c7
Refactor readFiles and displayFileContents functions to flatten outpu…
Mar 3, 2025
fe03c76
Fix readFiles function to remove trailing empty lines from file contents
Mar 3, 2025
9fb84ad
Refactor readFiles function to use extractLinesFromContent for improv…
Mar 3, 2025
2150905
Add option to number non-empty lines in cat implementation
Mar 3, 2025
2376e64
Update readFiles function to use object syntax for fs.readFile encodi…
Mar 3, 2025
24ceb1c
Implement ls command with options for single line output
Mar 3, 2025
d823cbe
Enhance ls command to accept multiple paths as arguments
Mar 3, 2025
97647ec
Add option to include hidden files in ls command
Mar 3, 2025
5cc3cac
Add option to display current and parent directories when including h…
Mar 3, 2025
6fe4d34
Remove unnecessary await from displayFileContents function call
Mar 3, 2025
80b1b24
Fix comment wording in extractLinesFromContent function for clarity
Mar 3, 2025
aee2490
Enhance documentation for implementLs function to clarify parameters …
Mar 3, 2025
cf568a4
Add wc command implementation to count lines, words, and characters i…
Mar 4, 2025
bad7699
Refactor wc command to handle multiple files and return line, word, a…
Mar 4, 2025
d8f858b
Refactor wc command to handle empty lines correctly and improve word …
Mar 4, 2025
1239f5f
Fix line count calculation in createLineWordsCharCountForFile function
Mar 4, 2025
0021d1e
Add aggregation of line, word, and character counts for multiple file…
Mar 4, 2025
5b3d5c9
Improve aggregation logic in wc command to handle single file cases
Mar 4, 2025
2b16d73
Refactor wc command to use commander for argument parsing
Mar 4, 2025
9ddebf4
Add line count option to wc command
Mar 4, 2025
e5ccf82
Add character count option to wc command
Mar 4, 2025
da2ec96
Add word count option to wc command
Mar 4, 2025
ea33cfd
Fix output logic in wc command to display total count when line optio…
Mar 4, 2025
cd2a6de
Update wc command to conditionally display total count for word option
Mar 4, 2025
3ffac88
Refactor wc command output logic to handle character option and displ…
Mar 4, 2025
8b23d95
Refactor output logic in wc command to use a helper function for form…
Mar 4, 2025
0e6ac90
Refactor output logic in wc command to inline option formatting
Mar 4, 2025
22fda0d
Rename function to count lines, words, and characters in file for cla…
Mar 4, 2025
f0b5293
Rename variables for clarity in line, word, and character counts in w…
Mar 4, 2025
8444b86
Rename function to improve clarity in file processing and output display
Mar 4, 2025
3d9756b
Refactor processFilesAndDisplayCounts to improve variable naming for …
Mar 4, 2025
c5e2b07
Add JSDoc comments to aggregateFileData function for improved documen…
Mar 4, 2025
faece38
rename file name to cat.js
Mar 4, 2025
b8fc18f
rename file name to ls.js
Mar 4, 2025
522e254
Refactor cat.js to streamline file reading and line display logic
Mar 5, 2025
e53d221
Rename variables for clarity and update function to print file conten…
Mar 5, 2025
19ed271
Refactor printFileWithLineNumbers function to improve line number han…
Mar 5, 2025
1517657
Rename options for line number handling in printFileWithLineNumbers f…
Mar 5, 2025
6aea96c
Refactor cat.js to rename and separate file reading and line printing…
Mar 5, 2025
7c4ca2a
Refactor ls.js to encapsulate directory listing logic in a dedicated …
Mar 5, 2025
d81c7d8
ignore .venv. folder
Mar 15, 2025
c180a48
Add commented debug log for command line arguments in cat.js
Mar 15, 2025
475f309
Remove unused import from cat.js
Mar 15, 2025
0ce2ebc
Use const instead of let for args in cat.js
eediallo Mar 19, 2025
bb95ea4
Remove commented debug log for command line arguments in cat.js
eediallo Mar 19, 2025
417bb33
Rename variable nOption to displayLineNumber for clarity in cat.js
eediallo Mar 19, 2025
9e94b59
Rename bOption to displayNonEmptyLineNumber for clarity in cat.js
eediallo Mar 19, 2025
148ddae
Refactor printLinesWithOptions to use ternary operators for cleaner l…
eediallo Mar 19, 2025
0f26dac
Remove unnecessary await from printLinesWithOptions for improved read…
eediallo Mar 19, 2025
b932eaf
Reintroduce function documentation for ls.js to clarify parameters an…
eediallo Mar 19, 2025
b75d6f6
Rename variables for clarity in ls.js and update logic to reflect new…
eediallo Mar 19, 2025
7ab2cca
Refactor listDirectoryContents in ls.js for improved readability
eediallo Mar 19, 2025
353f36b
Fix character count variable name and enhance output formatting in wc.js
eediallo Mar 19, 2025
17a4b2d
Remove unnecessary entry from .gitignore for cleaner configuration
eediallo Mar 19, 2025
d9d44a6
Simplify file content reading by removing the extractLinesFromContent…
eediallo Mar 19, 2025
cd0b63c
Remove unused function to handle empty line removal directly in wc.js
eediallo Mar 19, 2025
fcf9a71
Enhance output display in wc.js to include total counts for word opti…
eediallo Mar 19, 2025
05cef26
Refactor directory listing in ls.js to use forEach instead of map for…
eediallo Mar 22, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
node_modules
node_modules
40 changes: 40 additions & 0 deletions implement-shell-tools/cat/cat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { promises as fs } from "node:fs";
import process from "node:process";
import { program } from "commander";

program
.name("Implement cat")
.description("Implement a version of the cat program")
.option("-n, --number", "Number the output lines, starting at 1")
.option("-b, --number2", "Number the nonempty lines, starting at 1")
.argument("<paths...>", "The file paths to process")
.parse(process.argv);

const args = program.args;

const displayLineNumber = program.opts().number;
const displayNonEmptyLineNumber = program.opts().number2;
let lineNumber = 1;

async function readAndPrintFileContent(path) {
try {
const content = await fs.readFile(path, { encoding: "utf-8" });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: What does this do to the file from the filesystem? Could we do this more memory efficiently?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's reading the content of the file(path) provided through the CLI asyncronously which means it won't prevent other tasks from executing. Could we do this more memory efficiently? I am not 100% sure but I know if I was reading a large file, I would have used Stream to read it in chuncks instead of loading it once.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're along the right lines here, text files might be absolutely massive (e.g. logs) so making sure that our programs operate on them efficiently is really important, especially as we're processing line by line anyway

const lines = content.split("\n");
lines[lines.length - 1] === "" ? lines.pop() : lines;
printLinesWithOptions(lines);
} catch (err) {
console.error(err.message);
}
}

function printLinesWithOptions(lines) {
lines.forEach((line) => {
displayLineNumber
? console.log(`${lineNumber++} ${line}`)
: displayNonEmptyLineNumber && line !== ""
? console.log(`${lineNumber++} ${line}`)
: console.log(line);
});
}
// display file/s contents with line numbers.
args.map(readAndPrintFileContent);
51 changes: 51 additions & 0 deletions implement-shell-tools/ls/ls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import process from "node:process";
import { promises as fs } from "node:fs";
import { program } from "commander";

program
.name("Implement ls")
.description("Implements a version of the ls command")
.option("-1, --one", "Output all files and directories each in a line")
.option("-a, --hidden", "Output hidden files/directories")
.argument("[paths...]", "the paths to be processed")
.parse(process.argv);

/**
* Reads the contents of the specified directories and outputs the file names.
* Supports options for displaying hidden files and listing each file on a new line.
*
* @param {string[]} filePaths - The paths to be processed. Defaults to the current directory if no argument is provided.
* @param {boolean} one - If true, outputs each file and directory on a new line.
* @param {boolean} hidden - If true, includes hidden files and directories in the output.
*/

const filePaths = program.args.length ? program.args : ["."];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: This is a good way of implementing a sensible default, and shows that you know a great deal about how ls works


const outputOnePerLine = program.opts().one;
const includeHiddenFiles = program.opts().hidden;

async function listDirectoryContents(filePath) {
try {
const files = await fs.readdir(filePath, { withFileTypes: true }); // is returned as a Dirent
const output = [];

files.forEach((file) => {
if (includeHiddenFiles || !file.name.startsWith(".")) {
output.push(file.name);
}
});

if (includeHiddenFiles) {
output.unshift(".", "..");
}

outputOnePerLine === true
? output.forEach((file) => console.log(file))
: console.log(output.join(" "));
} catch (err) {
console.error(`Error reading directory ${filePath}:`, err.message);
}
}

//filePaths.map(listDirectoryContents);fi
filePaths.forEach(listDirectoryContents);
121 changes: 121 additions & 0 deletions implement-shell-tools/wc/wc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import { program } from "commander";
import process from "node:process";
import { promises as fs } from "node:fs";

program
.name("Implement wc")
.description("Implements a version of the wc command")
.option("-l, --line", "Counts file lines")
.option("-c, --char", "Counts characters in file")
.option("-w, --word", "Counts words in file")
.argument("[paths...]", "The path/s to process")
.parse(process.argv);

const args = program.args;

const lineOption = program.opts().line;
const charOption = program.opts().char;
const wordOption = program.opts().word;

async function countLinesWordsCharsInFile(path) {
const content = await fs.readFile(path, { encoding: "utf-8" });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: What are the tradeoffs of reading files in this way?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the file was large, that would caused high memory consumption and possible out of memory errors.


const lines = content.split("\n")
lines[lines.length - 1] === "" ? lines.pop() : lines;

const words = lines.flatMap((element) =>
element.split(" ").filter((word) => word !== "")
);

const chars = content.split("");

const numberOfLines = lines.length;
const numberOfWords = words.length;
const numberOfChars = chars.length;

if (lineOption && charOption) {
return `${numberOfLines} ${numberOfChars} ${path}`;
}

if (lineOption && wordOption) {
return `${numberOfLines} ${numberOfWords} ${path}`;
}

if (wordOption && charOption) {
return `${numberOfWords} ${numberOfChars} ${path}`;
}

if (lineOption) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: What if I'd like to have the lines and the chars?

Screenshot 2025-03-19 at 08 46 07

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I've not implemented this one. I am going to add the functionality

return `${numberOfLines} ${path}`;
}

if (charOption) {
return `${numberOfChars} ${path}`;
}

if (wordOption) {
return `${numberOfWords} ${path}`;
}

return `${numberOfLines} ${numberOfWords} ${numberOfChars} ${path}`;
}

async function processFilesAndDisplayCounts() {
const fileCounts = await Promise.all(args.map(countLinesWordsCharsInFile));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: awaiting and using Promise.all with a map here makes a lot of sense and makes sure we can process as many files as are needed, great stuff

fileCounts.forEach((fileCount) => console.log(fileCount));

const aggregatedFilesData = aggregateFileData(fileCounts);

if (aggregatedFilesData !== 0 && lineOption && !(wordOption || charOption)) {
console.log(`${aggregatedFilesData[0]} total`);
return;
}

if (aggregatedFilesData !== 0 && charOption && !(wordOption || lineOption)) {
console.log(`${aggregatedFilesData[0]} total`);
return;
}

if (aggregatedFilesData !== 0 && wordOption && !(lineOption || charOption)) {
console.log(`${aggregatedFilesData[0]} total`);
return;
}


if (
aggregatedFilesData !== 0 &&
((lineOption && charOption) || (lineOption && wordOption) || (wordOption && charOption))
) {
console.log(`${aggregatedFilesData[0]} ${aggregatedFilesData[1]} total`);
return;
}

aggregatedFilesData !== 0
? console.log(
`${aggregatedFilesData[0]} ${aggregatedFilesData[1]} ${aggregatedFilesData[2]} total`
)
: null;
}

/**
* Aggregates numerical data from an array of file content strings.
*
* @param {string[]} files - An array of strings, each representing the content of a file.
* Each string should contain space-separated numbers.
* @returns {number[]|number} - An array of sums for each column of numbers if there are multiple files,
* or 0 if there is only one file.
*/
function aggregateFileData(fileCounts) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Please could you talk me through this function line by line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure the aggregatedFilesData is an array with three possible values: sum of lines, chars or words. The following code:

 if (aggregatedFilesData !== 0 && lineOption && !(wordOption || charOption)) {
    console.log(`${aggregatedFilesData[0]} total`);
    return;
  }

whether aggregatedFilesData is not 0 and line option is provided but not the rest of the options(chars and words).if it is true, it means only sum of lines is available on aggregatedFilesData array therefore the sum is displayed with total in front of it.

it is the same for chars option in the following code:

if (aggregatedFilesData !== 0 && charOption && !(wordOption || lineOption)) {
    console.log(`${aggregatedFilesData[0]} total`);
    return;
  }

Here:

if (
    aggregatedFilesData !== 0 &&
    ((lineOption && charOption) || (lineOption && wordOption))
  ) {
    console.log(`${aggregatedFilesData[0]} ${aggregatedFilesData[1]} total`);
    return;
  }

This code checks if two options are provided, it could be line option and char option or line and word option. if it is true, I know the aggregatedFilesData array contains two values which are then displayed alongside the total string.

The return keyword in each statement ensure that the execution does not continue.

if the bove conditions are false: the sum of each option is display if aggregatedFilesData is not 0, hence the code below: aggregatedFilesData !== 0
? console.log(
${aggregatedFilesData[0]} ${aggregatedFilesData[1]} ${aggregatedFilesData[2]} total
)
: null;

const digits = fileCounts.map((element) =>
element.split(" ").slice(0, -1).map(Number)
);
const sums =
digits.length > 1
? digits[0].map((_, colIndex) =>
digits.reduce((sum, row) => sum + row[colIndex], 0)
)
: digits[0];
return sums;
}

processFilesAndDisplayCounts();
21 changes: 21 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "module",
"dependencies": {
"commander": "^13.1.0"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Including this as a dependency increases the portability of the code and made my life easier, thanks!

}
}