London | SDC-Nov-2025 | Ikenna Agulobi | Sprint 4 | Implement shell tools python by ike-agu · Pull Request #301 · CodeYourFuture/Module-Tools

ike-agu · 2025-12-12T12:59:37Z

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

This PR contains my implementation of cat, ls and wc shell tool commands in python

github-actions · 2025-12-12T12:59:57Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Sprint-4) doesn't match expected format (example: 'Sprint 2', without quotes)

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.

github-actions · 2025-12-12T13:00:08Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Sprint-4) doesn't match expected format (example: 'Sprint 2', without quotes)

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.

DaryaShirokova

Looks good! Just added a few comments

DaryaShirokova · 2026-01-10T19:06:29Z

implement-shell-tools/cat/README.md

+python cat.py sample-files/*.txt
+
+* Number every line across multiple files(including empty ones)
+python cat.py -n sample-files/*.txt


There is a small discrepancy in how you program handles some cases it seems:

$ cat -n 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 7 There's more to come, though... $ python cat.py -n sample-files/*.txt 1 Once upon a time... 2 3 There was a house made of gingerbread. 4 5 It looked delicious. 6 I was tempted to take a bite of it. 7 But this seemed like a bad idea... 8 9 There's more to come, though... 10

Thanks for pointing out the discrepancy. I’ve fixed the line-numbering logic so empty lines are handled correctly, matching cat behaviour. Thank you.

DaryaShirokova · 2026-01-10T19:07:36Z

implement-shell-tools/cat/cat.py

@@ -0,0 +1,50 @@
+import argparse
+# -------------------------------------------


It's ok to have comments, but it would be even better to have the code split into methods with descriptive names :)

Thanks for your review and suggestions. I’ve refactored my code by splitting it into small functions with descriptive names.

DaryaShirokova · 2026-01-10T19:09:03Z

implement-shell-tools/cat/cat.py

+        content = f.read()
+
+        lines = content.split("\n")


Maybe check if you could use readlines method here.

DaryaShirokova · 2026-01-10T19:15:20Z

implement-shell-tools/ls/ls_py.py

+    return args.paths
+
+# list a single directory
+def list_directory(directory_path, show_hidden, file_per_line):


It is good as is, but as a suggestion - a tiny bit cleaner approach would be to first get the list of directories/files and for list_directory function to just accept this list and print it (filtering whatever is not needed). This would follow 1 method/1 responsibility paradigm (which helps with code readability / reusability).

DaryaShirokova · 2026-01-10T19:16:38Z

implement-shell-tools/wc/wc.py

+
+
+def main():
+    # --------------------------------------------------------


Again, maybe try splitting into functions instead of comment blocks instead :)

I've refactored my code and removed comments. Thanks for feedback.

DaryaShirokova · 2026-01-10T19:18:28Z

implement-shell-tools/wc/wc.py

+    # --------------------------------------------------------
+    #  Print totals if there are multiple files
+    # --------------------------------------------------------
+    if len(args.paths) > 1:


Maybe there is a way to reuse the same functions (with different args) for printing individual lines and totals

DaryaShirokova · 2026-01-10T19:19:24Z

implement-shell-tools/wc/README.md

+python wc.py sample-files/*
+
+* Just lines
+python wc.py -l sample-files/3.txt


Looks like some outputs are different?

wc sample-files/* 1 4 20 sample-files/1.txt 1 7 39 sample-files/2.txt 5 24 125 sample-files/3.txt 7 35 184 total $ python wc.py sample-files/* 2 4 20 sample-files/1.txt 2 7 39 sample-files/2.txt 6 24 125 sample-files/3.txt 10 35 184 total

Hi @DaryaShirokova - thanks for pointing this out. The issue was caused because I used split("\n"), which overcounts when a file ends with a newline. I've now updated my implementation to use content.count("\n"), and the output now matches wc as expected.

…x wc

ike-agu · 2026-02-13T20:13:09Z

Hi @DaryaShirokova , apologies for circling back to this PR after such a long time. I should have addressed the feedback earlier.
Anyway!, Thank you for your review and feedback.
I’ve implemented all the requested changes.
When you have a moment, could you please take another look? Thank you!

DaryaShirokova · 2026-02-14T19:32:00Z

implement-shell-tools/wc/wc.py

+    if len(args.paths) > 1:
+        no_flags = not args.l and not args.w and not args.c
+
+        if no_flags:


I am not sure this is the correct logic here? e.g.

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

it still prints total with -l?

@DaryaShirokova, Thanks again for reviewing my PR. I’ve updated the logic so the total line now matches real wc behaviour. Please take a look when you have a moment. Thank you!

DaryaShirokova

LGTM overall, just one thing is still slightly different

DaryaShirokova · 2026-02-14T19:32:54Z

implement-shell-tools/wc/wc.py

+import re
+
+
+def main():


Optional: you could try to organize the code into multiple methods for readability

Print totals when multiple files are provided, even if -l, -w, or -c flags are used.This matches wc real behaviour.

ike-agu added 4 commits December 12, 2025 01:15

Add python implementation of catCommand and .gitignore file

1bce56a

Implement ls commands in python

4f141b3

Implement wc command-line tool in python

7a738ae

Fix comments

da1d1cc

ike-agu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

ike-agu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

DaryaShirokova added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 10, 2026

DaryaShirokova reviewed Jan 10, 2026

View reviewed changes

DaryaShirokova added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jan 10, 2026

ike-agu added 2 commits January 21, 2026 18:31

Fix line numbering to match cat -n and -b behaviour

0a84822

Refactor cat implementationinto functions for readability

4471a05

DaryaShirokova 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 Feb 1, 2026

Refactor wc into reusable function and fix line counting to match Uni…

ebd5f71

…x wc

DaryaShirokova added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 14, 2026

DaryaShirokova reviewed Feb 14, 2026

View reviewed changes

DaryaShirokova added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 14, 2026

Fix total line behavior to match wc for flagged multi-file input

62b3a4b

Print totals when multiple files are provided, even if -l, -w, or -c flags are used.This matches wc real behaviour.

ike-agu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 18, 2026

		@@ -0,0 +1,50 @@
		import argparse
		# -------------------------------------------



		def main():
		# --------------------------------------------------------

Uh oh!

Conversation

ike-agu commented Dec 12, 2025

Learners, PR Template

Changelist

Uh oh!

github-actions bot commented Dec 12, 2025

Uh oh!

github-actions bot commented Dec 12, 2025

Uh oh!

DaryaShirokova 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!

Choose a reason for hiding this comment

Uh oh!

ike-agu commented Feb 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaryaShirokova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments