Skip to content

Conversation

@HassanOHOsman
Copy link

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

This PR provides implementations of shell tools: cat, ls, and wc in Python.

…ent to diplay in one line with spaces in between
@HassanOHOsman HassanOHOsman 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 Jan 14, 2026
Copy link

@OracPrime OracPrime left a comment

Choose a reason for hiding this comment

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

You've made it very hard to review - that in itself is a problem. Don't commit individual file changes, commit self-consistent changes to your code. I should be able to pull any commit and it should work. I should be able to look at a single commit and see what the big change is.

At least one comment needs addressing - the double space in the ending.


for line in lines:
if args.n:
print(f"{line_number} {line}")

Choose a reason for hiding this comment

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

If the function of software is something else, then the formatting of the output may be less important. But the purpose of this software is to present information to the user, so it needed sorting

print(item)
else:
print(item, end=" ")
print(item, end=" ")

Choose a reason for hiding this comment

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

Why two spaces? Should it be \t? I don't actually know, but I think you should find out :)

Copy link
Author

Choose a reason for hiding this comment

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

replaced the double spacing with /t. Tested it and worked. Thank you!



def process_file(file_path):
global total_lines, total_words, total_characters, file_count

Choose a reason for hiding this comment

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

global is rarely a good word to see in source code... what's it doing here?

Copy link
Author

Choose a reason for hiding this comment

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

the goal of having it so that total_lines, total_words, total_characters, file_count can be used inside my helper function otherwise, an error would've been thrown for them not being defined locally. Anyway, I am looking for alternatives in the meantime.

Choose a reason for hiding this comment

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

A function can return more than one value. You can define them in the function and them have them all on the return statement. For example

def fun():
return "stuff", 20

s, x = fun()
print(s)
print(x)


if args.line:
print(f"{stats['lines']} {path}")
print(f"{stats['lines']} {file_path}}")

Choose a reason for hiding this comment

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

Unless I'm seeing some distorted github view of reality, it looks like you are committing individual file changes as one commit each. You don't want to do that. Each commit should be a group of file changes which is logically consistent.

Copy link
Author

Choose a reason for hiding this comment

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

My bad, I thought it frequent committing is the way. I will aim to group my commits moving forward. Thank you!

Choose a reason for hiding this comment

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

Frequent commits are good, but every commit should be a logical change: the commit description should describe the group of changes being made, but it's all the changes need for a particular step forward, not file by file

Copy link
Author

Choose a reason for hiding this comment

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

Thank you!


parser.add_argument("-1", "--one", dest="one", help="Output one entry per line", action="store_true")
parser.add_argument("-a", help="List all files & directories, including hidden ones", action="store_true")
parser.add_argument("paths", nargs="*", default=["."], help="The file path to read from")

Choose a reason for hiding this comment

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

Arg is paths, comment is path. That will confuse people.

Copy link
Author

Choose a reason for hiding this comment

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

Reworded the confusing bit. Hope it's good now.

@OracPrime OracPrime 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 Jan 14, 2026
@HassanOHOsman HassanOHOsman 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 Jan 15, 2026
Copy link

@OracPrime OracPrime left a comment

Choose a reason for hiding this comment

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

Nice work. I think you're done with this one now!

@OracPrime OracPrime added Reviewed Volunteer to add when completing a review with trainee action still to take. 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 Jan 15, 2026
@HassanOHOsman HassanOHOsman removed the Reviewed Volunteer to add when completing a review with trainee action still to take. label Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 4 Assigned during Sprint 4 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants