Skip to content

London | 26-March-SDC | Ammad Ur Rehman | Sprint 4 | Implement shell tools (cat, ls, wc) in Python - #540

Open
anosidium wants to merge 14 commits into
CodeYourFuture:mainfrom
anosidium:implement-shell-tools-in-python
Open

London | 26-March-SDC | Ammad Ur Rehman | Sprint 4 | Implement shell tools (cat, ls, wc) in Python#540
anosidium wants to merge 14 commits into
CodeYourFuture:mainfrom
anosidium:implement-shell-tools-in-python

Conversation

@anosidium

@anosidium anosidium commented Jun 15, 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

Reimplement the shell programs as a Python program.

@anosidium anosidium added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jun 15, 2026
@SlideGauge SlideGauge added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jun 20, 2026
SlideGauge
SlideGauge reviewed Jun 20, 2026
View reviewed changes

@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, left several comments, could you fix them please?

Comment thread implement-shell-tools/cat/cat.py Outdated
continue

with open(path, "r") as file:
line_number = 1

@SlideGauge SlideGauge Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

README requires cat -n sample-files/*.txt to behave like real cat -n, which numbers lines continuously across all files

@anosidium anosidium Jun 24, 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.

Real cat produces:

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

Python cat produces:

python3 cat.py -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...

The only difference is the indentation.

Comment thread implement-shell-tools/cat/cat.py Outdated
Comment thread implement-shell-tools/cat/cat.py Outdated
Comment thread implement-shell-tools/ls/ls.py Outdated

args = parser.parse_args()

entries = os.listdir(args.filepath)

@SlideGauge SlideGauge Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does os.listdir return dir list in an alphabetical order? Compare with what real ls does

@anosidium anosidium Jun 25, 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.

When I run ls -a, it produces:

.               ls.py           README.md
..              node_modules    sample-files

When I run the Python variant, python3 ls.py -a:

.       ..      README.md       ls.py   node_modules    sample-files

The alphabetically ordering does not match.

Comment thread implement-shell-tools/ls/ls.py
@anosidium

anosidium commented Jun 25, 2026

Copy link
Copy Markdown
Author

Thanks for the review. I've made some changes.

@anosidium anosidium removed the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Jun 25, 2026
SlideGauge
SlideGauge reviewed Jul 26, 2026
View reviewed changes
Comment thread implement-shell-tools/cat/cat.py

try:
with open(path, "r") as file:
line_number = 1

@SlideGauge SlideGauge Jul 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

numbering still restarts per file, not continuous.

@anosidium anosidium Aug 31, 2026 •
edited
Loading

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.

Sorry, I’m going to have to disagree with this one. The program matches the behaviour of cat on my system. I checked all the commands in README.md and the output matches.

If I move line_number = 1 outside the loop, the numbering becomes continuous across files and no longer matches cat.

My Python implementation:

 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
 7  There's more to come, though...

cat:

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

So keeping line_number = 1 inside the per-file loop is what makes my implementation match cat on macOS. This may be a GNU/Linux vs BSD/macOS difference.

@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 Jul 26, 2026
@anosidium anosidium 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 31, 2026
@anosidium

anosidium commented Sep 15, 2026

Copy link
Copy Markdown
Author

@cjyuan Could you please review my pull request? Thank you.

@anosidium

anosidium commented Sep 16, 2026

Copy link
Copy Markdown
Author

@SlideGauge Could you please review my pull request? Thank you.

@cjyuan

cjyuan commented Sep 30, 2026

Copy link
Copy Markdown

Note: It's an GNU/Linux vs BSD/macOS issue. The curriculum team said for the past cohort, either version is acceptable. So I am marking this PR as "Complete".

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Sep 30, 2026
@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown

Your PR didn't include a Task ID in its description.

Make sure you have put the correct Task ID in its description.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

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

Reviewers

1 more reviewer
@SlideGauge SlideGauge SlideGauge left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@anosidium @cjyuan @SlideGauge