-
-
Notifications
You must be signed in to change notification settings - Fork 27
London | 25-SDC-July | Andrei Filippov | Sprint 4 | Implement shell tools in Python #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
London | 25-SDC-July | Andrei Filippov | Sprint 4 | Implement shell tools in Python #142
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start on these implementations - I've left a comment on each file with a pointer to show you where you could improve them.
For your question about formatters what do you mean - formatting your code or formatting strings?
- You can use any code formatter (or none at all), as long as you are consistent with the code you commit
- Python has a few ways to do string formatting, the newest versions of python have something called "f-strings" that are very powerful, but might take a lot of time to learn fully
for path in args.path: | ||
with open(path) as file: | ||
lines = file.readlines() | ||
line_num = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When printing out the line numbers, compare how the original cat application works to yours. Do you see any discrepancies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only discrepancies I see are in spacing. Is that acceptable, provided the output is correct?
implement-shell-tools/ls/ls.py
Outdated
path_proceeding(args.path) | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very minor remark, but try not to leave lots of blank lines throughout the file, like here at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this
total.append(output.copy()) | ||
output.append(path) | ||
string_list = map(str, output) | ||
print(" ".join(string_list)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you see any problem that might happen with the displayed spacing of the output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly, no
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- For cat, try running the original cat program with the
-n
argument, with multiple files using a wildcard, and then run your own. Can you see any differences? - Can you see any issues with the alignment of columns if you run ẁc`with multiple files using a wildcard?
Learners, PR Template
Self checklist
Changelist
Implemented shell tools in Python
Questions
what formatter to use for python?