Skip to content

LONDON | 26-SDC-JUly | Boshra Mahmoudi | Sprint 4 | Implement shell tools in python - #678

Open
BoshraM wants to merge 19 commits into
CodeYourFuture:mainfrom
BoshraM:12-implement-shell-tools-cat-ls-wc-in-python
Open

LONDON | 26-SDC-JUly | Boshra Mahmoudi | Sprint 4 | Implement shell tools in python#678
BoshraM wants to merge 19 commits into
CodeYourFuture:mainfrom
BoshraM:12-implement-shell-tools-cat-ls-wc-in-python

Conversation

@BoshraM

@BoshraM BoshraM commented Aug 31, 2026

Copy link
Copy Markdown
  • 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

Task code

CYF-1152

@BoshraM BoshraM added 📅 Sprint 4 Assigned during Sprint 4 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Tools The name of the module. labels Aug 31, 2026

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

Mandatory fix

  • ls.py

Great work!

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

for filename in args.files:

with open(filename) as file:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if filename doesn't exist? You might try with error handling for FileNotFoundError.

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.

@Khantdotcom I added try and except so it can show an error message when the file does not exist.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work. @BoshraM

for filename in args.files:

with open(filename, "rb") as file:
content = file.read()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reading the whole file with .read() is putting that data into RAM. What if you point this at a 50GB log file? Check out this code for learning a better approach here

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.

@Khantdotcom I didn’t have any clue about this. Thanks, I’ll use this approach next time.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@BoshraM It's completely okay with your current approach! The whole idea of suggested one is just chunking, which means reading the file in part by part. Hope you learned something.

print(file, end=" ")
print()

else:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

what happens if we try like python ls.py completely_fake_file.txt?

Your current condition checks if it's a directory but what if it is a file. Shall we just print or do something with that condition?

And what if it's neither of dir nor file? How do we handle that?

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.

@Khantdotcom thanks, I didn’t pay attention to that issue. I added another check so it now handles existing files, directories, and nonexistent paths.

@abdishakoor-dev abdishakoor-dev 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 Sep 8, 2026
@abdishakoor-dev

Copy link
Copy Markdown

@Khantdotcom I have switched labels from Needs Review to Reviewed. I think you have forgotten to do this :)

@Khantdotcom

Copy link
Copy Markdown

@abdishakoor-dev thanks for noticing that. I forgot to ask admin about that label settings. Somehow I cannot edit labels in this Module-Tools. What do you suggest me to do?

@abdishakoor-dev

Copy link
Copy Markdown

@Khantdotcom do you have access to the Slack code review channel? Can you post on their please so this issue is picked up by the team.

@Khantdotcom

Copy link
Copy Markdown

@Khantdotcom do you have access to the Slack code review channel? Can you post on their please so this issue is picked up by the team.

Thanks, appreciate your help.

@Khantdotcom

Copy link
Copy Markdown

Marked the label as "Reviewed" and "Complete"

@Khantdotcom Khantdotcom added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Sep 9, 2026
@abdishakoor-dev abdishakoor-dev added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed 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. labels Sep 9, 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. Module-Tools The name of the module. 📅 Sprint 4 Assigned during Sprint 4 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants