London | 26-SDC-Mar | Khilola Rustamova| Sprint 4 |Implement shell tools python by HilolaRustam · Pull Request #537 · CodeYourFuture/Module-Tools

HilolaRustam · 2026-06-10T14:00:31Z

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

implementation of cat, ls and wc with python

illicitonion

This is generally looking good, and like things work :) I left a few style comments, and many of my comments to one project apply to the others as well.

illicitonion · 2026-06-10T17:31:50Z

+    return sorted(files)
+
+def read_lines(file):
+           with open(file, "r", encoding="utf-8") as f:


This block is more indented than the others - why is that?

That was an accident, i have aligned it.

illicitonion · 2026-06-10T17:33:03Z

+        try:
+            lines = read_lines(file)
+        except FileNotFoundError:
+            print(f"cat: {file}: No such file or directory", file=sys.stderr)


Good job printing to stderr not stdout :)

However! If you ran into an issue here, I think your program will still exit with exit code 0. What exit code do you think it should exit with?

You're right. A missing file should result in a non-zero exit code to indicate an error. I've updated the code to exit with code 1 if any file cannot be read.

illicitonion · 2026-06-10T17:34:39Z

+            sys.stdout.write(prefix + line)
+
+
+def main():


This all works, but often people will have their main function at the top of the file so it's the first thing you read, and allow you to scroll down to read details if needed. You don't need to follow this, but it can often make it easier to read/review code.

Thank you, I've moved main() higher in the file to make the program flow easier to see.

illicitonion · 2026-06-10T17:35:20Z

+            paths.append(a)
+
+    files = expand_paths(paths)
+    print_lines(files, number_all,number_nonempty)


I've noticed your whitespace is a bit inconsistent (e.g. no space after this ,) - you may want investigate a tool like black to help automate fixing this.

Thank you for your suggestion. I've cleaned up the whitespace issues and I'll take a look at Black for automatic formatting

illicitonion · 2026-06-10T17:49:52Z

+            number_all = True
+        elif a == "-b":
+            number_nonempty = True
+            number_all = False #-b overrides -n


What would happen if -b appeared before -n in the command line?

I've changed the flag handling so -b always takes precedence regardless of argument order.

illicitonion · 2026-06-10T17:51:01Z

+            paths.append(a)
+
+    files = expand_paths(paths)
+    print_lines(files, number_all,number_nonempty)


We often used named arguments in Python when we have multiple arguments of the same type, to make sure we don't accidentally pass them in the wrong order. If you wrote print_lines(files, number_nonempty, number_all) here it wouldn't be obvious that the order was swapped. Instead we can write:

print_lines(files, number_all=number_all, number_nonempty=number_nonempty)

and if we swapped them by mistake it would be obvious.

That makes sense. I've updated the call to use named arguments

illicitonion · 2026-06-10T17:51:18Z

+def list_dir(path, show_all=False, one_per_line = False):
+    try:
+        entries = os.listdir(path)
+    except FileNotFoundError:


Same question as above about exit codes.

illicitonion

This looks really good, well done!

illicitonion · 2026-06-11T12:48:12Z

+    for a in args:
+        if a == "-n":
+            number_all = True
+        elif a == "-b":
+            number_nonempty = True
+        else:
+            paths.append(a)
+
+    if number_nonempty:
+        number_all = False        


This kind of relationship where different options/states are related but exclusive can be hard to follow. (Here, it should never be the case that number_all and number_nonempty are both True - it's an invalid state in the program).

Instead of this, we sometimes use enums for this (or can use strings as enums). Consider a single variable number_mode which could be set to either "none", "non_empty" or "all" - here we don't need to think about what both True mean - we just have one variable which could have one of three variables. What do you think about this?

illicitonion · 2026-06-11T12:51:34Z

+
+    return line_count, word_count, byte_count
+
+def expand(paths):


In general you shouldn't need to do this kind of glob expansion yourself - the shell should do this for you and expand the globs into different file names when calling your program. You can test this out by printing what sys.argv is when you run python3 my-wc.py some*glob - look at what arguments you actually get :)

illicitonion · 2026-06-11T12:52:00Z

+        try:
+            l, w, c = count_file(file)
+        except FileNotFoundError:
+            print(f"wc: {file}: No such file or directory", file=sys.stderr)


What exit code will you exit with if you failed to read one file?

HilolaRustam added 5 commits June 9, 2026 10:43

created files for shell tools with python

60d6701

implementation of cat with python

aee68b7

implementation of ls with python

80ca854

implementation of wc with python

1e3fc25

Merge branch 'main' into implement-shell-tools-python

ffb4416

HilolaRustam added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Tools The name of the module. 📅 Sprint 3 Assigned during Sprint 3 of this module labels Jun 10, 2026

illicitonion 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 Jun 10, 2026

illicitonion reviewed Jun 10, 2026

View reviewed changes

illicitonion 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 Jun 10, 2026

HilolaRustam added 3 commits June 11, 2026 11:13

error handling corrected, structure is changed, indentation fixed

efadbb0

corrected error handling issue and moved the main up.

ad6883f

small changes

db7c86a

HilolaRustam 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 Jun 11, 2026

illicitonion reviewed Jun 11, 2026

View reviewed changes

illicitonion 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 Jun 11, 2026

Uh oh!

Conversation

HilolaRustam commented Jun 10, 2026 • edited by illicitonion Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HilolaRustam commented Jun 10, 2026 •

edited by illicitonion

Loading