Skip to content

London | 26-SDC-July | Raihan Sharif | Sprint 4 | implement shell tools python - #649

Open
RaihanSharif wants to merge 19 commits into
CodeYourFuture:mainfrom
RaihanSharif:shell-tools-python
Open

London | 26-SDC-July | Raihan Sharif | Sprint 4 | implement shell tools python#649
RaihanSharif wants to merge 19 commits into
CodeYourFuture:mainfrom
RaihanSharif:shell-tools-python

Conversation

@RaihanSharif

@RaihanSharif RaihanSharif commented Aug 4, 2026

Copy link
Copy Markdown

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

Implemented cat, ls, wc.

Task code: CYF-1152

@RaihanSharif RaihanSharif added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 5, 2026

@SlideGauge SlideGauge 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.

Good job, I left several comments, could you fix them please?

prog="a simple version of wc. Takes in one or more files.",
description="ls command line tool which can accept -l -w -c cflags")

parser.add_argument("-l", action="store_true", help="show line count", default="l")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Each flag is declared action="store_true", default="l" (and "w", "c"). With store_true, when the flag is
absent the value falls back to that default string - and a non-empty string is truthy. So run wc -l
sample-files/3.txt: does it print only the line count, or all three? What should default be for a boolean flag so
that "flag not given" reads as false?

@RaihanSharif RaihanSharif Aug 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

...uh, yeah, that's bad.

I've now added a check that if no flags are supplied, then just set them all to true.
Which then means that wc prints the lines, word count, and bytes of the files.

Otherwise, the flags are processed individually, so that any combination of the three will work.

# {some_str:6} left justifed string, length fo at least 6
output.append(f"{line_num:6}\t{line}")
elif number_all:
for i, line in enumerate(lines, start=1):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

format_lines numbers with enumerate(..., start=1) and is called fresh for each file in cat_file. Run cat -n
sample-files/*.txt against the real cat - does the count reset to 1 at each file, or keep climbing? Where would the counter need to live to carry across files?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, the numbering starts from 1 for each file in the cat input

cat -n sample-files/*
     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...

python cat:

python3 cat.py -n sample-files/*
     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...

You asked this in the nodeJS version of cat as well. Are you hinting at something, and I'm just not getting it?

Comment thread implement-shell-tools/wc/wc.py
lines = file.readlines()

if (args.l):
if (len(lines) > 0 and lines[-1] == ""):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lines[-1] == "" - but readlines() keeps the trailing \n, so an element is "\n", never "". Does this pop ever
actually run? Check wc -l against real wc on a file that does and doesn't end in a newline.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for pointing that out, I 'translated' my js code too literally without accounting for how readlines() or how wc actually counts lines.

wc counts the number of '\n' characters, not necessarily non-empty lines. Last empty line is excluded because it doesn't have a '\n' in it.

I've changed my implementation to just read the file, and count the number of '\n' characters. The output now matches the real wc.

It also made my -w logic a bit simpler.

Comment thread implement-shell-tools/ls/requirements.txt
Comment thread implement-shell-tools/wc/requirements.txt
Comment thread implement-shell-tools/cat/cat.py
@SlideGauge SlideGauge added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 5, 2026
@RaihanSharif RaihanSharif added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants