-
-
Notifications
You must be signed in to change notification settings - Fork 107
London | 26-SDC-July | Alex Jamshidi | Sprint 4 | Implement shell tools in python #676
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?
Changes from all commits
78bdfd5
60b323e
3a35a7a
085224e
b28f41d
7d6a298
fce72f0
14d4d33
2b981ed
fa7436b
d4720b1
5c64429
769706f
65e60ae
efdd20b
61a6eaa
eeb4d7a
5455734
a0572e4
0cb7c2e
696e335
68d08db
27ffc84
0e0f38b
6b73659
eb70f3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| .venv |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import argparse | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| # ===== Argument Handling ===== | ||
| parser = argparse.ArgumentParser( | ||
| prog="cat", | ||
| description="Prints file content", | ||
| ) | ||
|
|
||
| parser.add_argument("-b", action="store_true", help="Numbers lines that aren't empty") | ||
| parser.add_argument("-n", action="store_true", help="Numbers all lines") | ||
| parser.add_argument("file_names", nargs="*", help="Files for which to display content") | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| # ===== cat Procedure ===== | ||
| def cat(args): | ||
| cwd = os.getcwd() | ||
| all_files_contents = read_files(args.file_names, cwd) | ||
| execute_flags(all_files_contents) | ||
| print_lines(all_files_contents) | ||
|
|
||
| # ===== Extracting Data from Arguments ===== | ||
| def read_files(file_names, cwd): | ||
| all_files_contents = [] | ||
|
|
||
| for file_name in file_names: | ||
| file_content = read_file(file_name, cwd) | ||
| all_files_contents.append(file_content.splitlines()) | ||
| return all_files_contents | ||
|
|
||
| def read_file(file_name, cwd): | ||
| file_path = Path(cwd) / file_name | ||
| with open(file_path, "r", encoding="utf-8") as f: | ||
| return f.read().rstrip() | ||
|
|
||
| # ===== Flag Handling ===== | ||
| def execute_flags(all_files_contents): | ||
| if args.b: | ||
| for file_content in all_files_contents: | ||
| line_number = 1 | ||
| for line_idx, line in enumerate(file_content): | ||
| if line != "": | ||
| file_content[line_idx] = f"{line_number:>6}\t{line}" | ||
| line_number += 1 | ||
|
|
||
| elif args.n: | ||
| for file_idx, file_content in enumerate(all_files_contents): | ||
| all_files_contents[file_idx] = [ | ||
| f"{line_idx:>6}\t{line}" | ||
| for line_idx, line in enumerate(file_content, start=1) | ||
| ] | ||
|
|
||
| def make_list(output_string): | ||
| return output_string.replace("\t", "\n").replace("\n\n", "\n") | ||
|
|
||
| # ===== Print Output ===== | ||
| def print_lines(all_files_contents): | ||
| for file_content in all_files_contents: | ||
| for line in file_content: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason to use two loops here? Could your data structure be simplified?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment. |
||
| print(line) | ||
|
|
||
| # ===== Run cat ===== | ||
| cat(parser.parse_args()) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| import argparse | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| # Argument Handling | ||
| parser = argparse.ArgumentParser( | ||
| prog="ls", | ||
| description="Print line, word, and byte counts for each file.", | ||
| ) | ||
|
|
||
| parser.add_argument("-1", action="store_true", help="Show output on separate lines") | ||
| parser.add_argument("-a", action="store_true", help="Show hidden files") | ||
| parser.add_argument("file_system_items", nargs="*", help="Files or folders to display") | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| # ===== ls Procedure ===== | ||
| def ls(args): | ||
| flag_status = {"print_in_list": False, "show_all": False} | ||
| execute_flags(flag_status) | ||
|
|
||
| cwd = os.getcwd() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you use the cwd for in this program? |
||
| fs_items = check_args_length(args.file_system_items) | ||
| dir_args = get_dir_args(cwd, fs_items) | ||
| file_args = get_file_args(cwd, fs_items, flag_status) | ||
|
|
||
| print_output(populate_output(cwd, fs_items, dir_args, file_args, flag_status), flag_status) | ||
|
|
||
| # ===== Flag Handling ===== | ||
| def execute_flags(flag_status): | ||
| if getattr(args, "1"): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, when you are doing things like |
||
| flag_status["print_in_list"] = True | ||
| if args.a: | ||
| flag_status["show_all"] = True | ||
|
|
||
| def make_list(output_string): | ||
| return output_string.replace("\t", "\n").replace("\n\n", "\n") | ||
|
|
||
| # Extracting Data from Arguments | ||
| def check_args_length(fs_items): | ||
| if len(fs_items) == 0: | ||
| fs_items.append(".") | ||
| return fs_items | ||
|
|
||
| def get_dir_args(cwd, fs_items): | ||
| return [ | ||
| p | ||
| for p in fs_items | ||
| if (Path(cwd) / p).is_dir() | ||
| ] | ||
|
|
||
| def get_file_args(cwd, fs_items, flag_status): | ||
| file_args = [ | ||
| p | ||
| for p in fs_items | ||
| if not (Path(cwd) / p).is_dir() | ||
| ] | ||
| if not flag_status["show_all"]: | ||
| file_args = remove_dot_files(file_args) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are multiple places where you use remove_dot_files in this program. Is there a reason for that? |
||
| return file_args | ||
|
|
||
| # Populating and Outputting Data | ||
| def populate_output(cwd, fs_items, dir_args, file_args, flag_status): | ||
| output_string = "" | ||
|
|
||
| if len(fs_items) == 1: | ||
| for file in file_args: | ||
| output_string += file + " " | ||
| for dir_item in dir_args: | ||
| output_string += dir_output(dir_item, cwd, flag_status) | ||
| else: | ||
| for file in file_args: | ||
| output_string += file + "\t" | ||
| for dir_item in dir_args: | ||
| output_string += f"\n\n{dir_item}:\n" + dir_output(dir_item, cwd, flag_status) | ||
| return output_string | ||
|
|
||
| def dir_output(dir_item, cwd, flag_status): | ||
| target_path = Path(cwd) / dir_item | ||
| contents = os.listdir(target_path) | ||
| output_str = "" | ||
| if flag_status["show_all"]: | ||
| output_str += ".\t..\t" | ||
| else: | ||
| contents = remove_dot_files(contents) | ||
|
|
||
| for item in contents: | ||
| output_str += item + "\t" | ||
| return output_str | ||
|
|
||
| def remove_dot_files(file_list): | ||
| return [item for item in file_list if not item.startswith(".")] | ||
|
|
||
| def print_output(output_string, flag_status): | ||
| output = output_string | ||
| if flag_status["print_in_list"]: | ||
| output = make_list(output) | ||
| print(output.rstrip()) | ||
|
|
||
| # ===== Run ls ===== | ||
| ls(parser.parse_args()) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| import argparse | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| # Argument Handling | ||
| parser = argparse.ArgumentParser( | ||
| prog="wc", | ||
| description="Print line, word, and byte counts for each file.", | ||
| ) | ||
|
|
||
| parser.add_argument("-l", action="store_true", help="Show line count") | ||
| parser.add_argument("-w", action="store_true", help="Show word count") | ||
| parser.add_argument("-c", action="store_true", help="Show byte size") | ||
| parser.add_argument("files", nargs="*", help="File names to process") | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
| # Global Variables | ||
| metrics = ["line_count", "word_count", "byte_size"] | ||
| displayed_metrics = [] | ||
|
|
||
| # ===== wc Procedure ===== | ||
| def wc(args): | ||
| cwd = os.getcwd() | ||
| file_names = args.files | ||
|
|
||
| execute_flags() | ||
| all_files_data = add_totals(extract_files_data(file_names, cwd)) | ||
| print_output(all_files_data) | ||
|
|
||
| # ===== Flag Handling ===== | ||
| def execute_flags(): | ||
| if args.w: | ||
| displayed_metrics.append("word_count") | ||
| if args.l: | ||
| displayed_metrics.append("line_count") | ||
| if args.c: | ||
| displayed_metrics.append("byte_size") | ||
|
|
||
| # ===== Extracting Files Data ===== | ||
| def extract_files_data(file_names, cwd): | ||
| all_files_data = [] | ||
|
|
||
| for file_name in file_names: | ||
| file_data = {} | ||
| file_data["name"] = file_name | ||
| file_data["text"] = read_file(file_name, cwd) | ||
| file_data["line_count"] = calculate_line_count(file_data["text"]) | ||
| file_data["word_count"] = calculate_word_count(file_data["text"]) | ||
| file_data["byte_size"] = read_byte_size(file_name, cwd) | ||
|
|
||
| all_files_data.append(file_data) | ||
|
|
||
| return all_files_data | ||
|
|
||
| def read_file(file_name, cwd): | ||
| file_path = Path(cwd) / file_name | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you using the |
||
| return file_path.read_text(encoding="utf-8").rstrip() | ||
|
|
||
| def calculate_line_count(text): | ||
| if not text: | ||
| return 0 | ||
| return len(text.splitlines()) | ||
|
|
||
| def calculate_word_count(text): | ||
| return len(text.split()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason the word count method has a different structure to your line count method? |
||
|
|
||
| def read_byte_size(file_name, cwd): | ||
| file_path = os.path.join(cwd, file_name) | ||
| return os.path.getsize(file_path) | ||
|
|
||
| def add_totals(all_files_data): | ||
| if len(all_files_data) <= 1: | ||
| return all_files_data | ||
|
|
||
| totals_data = {"name": "total"} | ||
|
|
||
| for metric in metrics: | ||
| metric_sum = 0 | ||
| for file in all_files_data: | ||
| metric_sum += file[metric] | ||
| totals_data[metric] = metric_sum | ||
|
|
||
| all_files_data.append(totals_data) | ||
| return all_files_data | ||
|
|
||
| # ===== Outputting Data ===== | ||
| def print_output(output_data): | ||
| for file in output_data: | ||
| output_string = "" | ||
|
|
||
| for metric in metrics: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to loop over metrics and then check if it is in displayed_metrics? Could you simplify this? |
||
| if not displayed_metrics or metric in displayed_metrics: | ||
| output_string += str(file[metric]).rjust(8) | ||
|
|
||
| output_string += f" {file['name']}" | ||
| print(output_string) | ||
|
|
||
| # ===== Run wc ===== | ||
| wc(parser.parse_args()) | ||
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.
Here it looks like you loop over all the content twice.Once to add the line numbers, then once again to print. Is it possible to do this only looking at the data once?
Uh oh!
There was an error while loading. Please reload this page.
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.
There is a reason for this. The data in all_file_contents is stored as two lots of nested arrays.
For 3 files it might look like this:
[[line, line, line, line], [line, line, line], [line, line, line, line]]
For a single file it looks like this:
[[line, line, line]]
So the first loop essentially just unwraps that first array.
I could update the code in read_files() to unpack all lines into a single array, even if there are multiple files... that would be something like...
(replacing lines 29 and 30).
This would then let me have a single loop in each of the two locations you flagged:
for line in all_file_contents:
But I chose to do it this way to preserve the information of which lines are from which files. The reason to do this was because, this version of cat implements only 2 flags, but was written with the view to be potentially expanded, so I wanted to maintain as much information about the input as possible, letting the print function unpack it all at the end.
This is also why there are two loops below.