Skip to content

Barcelona | OTC-2025-2 | Sahim Saami | Sprint 4 | Implement shell tools - #7

Open
sahimsaami wants to merge 5 commits into
mainfrom
implement-shell-tools
Open

Barcelona | OTC-2025-2 | Sahim Saami | Sprint 4 | Implement shell tools#7
sahimsaami wants to merge 5 commits into
mainfrom
implement-shell-tools

Conversation

@sahimsaami

Copy link
Copy Markdown
Owner

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

const folder = args.find(a => !a.startsWith("-")) || ".";

let files = fs.readdirSync(dir);
let files = fs.readdirSync(folder);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ls output is typically sorted. You should apply: files.sort();

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thanks,
added the files.sort() and its working

Comment thread implement-shell-tools/wc/wc.js Outdated

const lines = text.split("\n").length;
const words = text.trim().split(/\s+/).filter(Boolean).length;
const words = text.trim().split(/\s+/).length;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the file is empty, this returns 1 instead of 0 because split() on an empty string still produces [""].
maybe const words = text.trim() ? text.trim().split(/\s+/).length : 0;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

fix: handle empty files correctly in wc

@yassineyahya-occ yassineyahya-occ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall structure is clean and readable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants