Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
node_modules
.venv
50 changes: 50 additions & 0 deletions implement-shell-tools/cat/cat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import argparse
import sys


def parse_args():
parser = argparse.ArgumentParser(
description="Reads file(s) and writes them to the standard output",
)
parser.add_argument("paths", nargs="+", help="The file path(s) to process")
parser.add_argument(
"-n",
action="store_true",
dest="number_all",
help="Number the output lines, starting at 1.",
)
parser.add_argument(
"-b",
action="store_true",
dest="number_nonblank",
help="Number only non-blank output lines, starting at 1.",
)
return parser.parse_args()


def main():
args = parse_args()

try:
for path in args.paths:
line_num = 1

with open(path, "r", encoding="utf-8") as file:
for raw_line in file:
line = raw_line.rstrip("\n")
is_blank = line.strip() == ""
should_number = args.number_all or (
args.number_nonblank and not is_blank)

if should_number:
print(f"{line_num} {line}")
line_num += 1
else:
print(line)
except OSError as err:
print(err, file=sys.stderr)

return 0


main()
86 changes: 86 additions & 0 deletions implement-shell-tools/ls/ls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import argparse
import os
import stat
import sys


def parse_args():
parser = argparse.ArgumentParser(
description="List directory contents",
)
parser.add_argument(
"paths",
nargs="*",
help="The file path to process (defaults to current directory)",
)
parser.add_argument(
"-a",
action="store_true",
dest="include_hidden",
help="Include directory entries whose names begin with a dot ('.').",
)
parser.add_argument(
"-1",
action="store_true",
dest="one_per_line",
help="Force output to be one entry per line.",
)
return parser.parse_args()


def filter_hidden(files: list[str]) -> list[str]:
return [name for name in files if not name.startswith(".")]


def get_visible_entries(files: list[str], include_hidden: bool):
return files if include_hidden else filter_hidden(files)


def format_entries(files: list[str], one_per_line: bool):
if len(files) == 0:
return
print(("\n" if one_per_line else "\t").join(files))


def main():
args = parse_args()

try:
file_paths = args.paths if args.paths else ["."]
include_hidden = bool(args.include_hidden)
one_per_line = bool(args.one_per_line)

result_files: list[str] = []
result_dirs: dict[str, list[str]] = {}

for file_path in file_paths:
st = os.stat(file_path)
# Is a file?
if stat.S_ISREG(st.st_mode):
result_files.append(file_path)
# Is a directory?
if stat.S_ISDIR(st.st_mode):
result_dirs[file_path] = os.listdir(file_path)
Comment on lines +57 to +63

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

On these two lines you added comments # Is a file? and # Is a directory?. In this case, the code right below (stat.S_ISREG and stat.S_ISDIR) is already quite clear about what it’s doing, so the comments don’t add much information. Over time, comments that merely restate the code can become noise and make it harder to spot the comments that do carry important context (like explaining a tricky edge case or a non-obvious design decision).

How might you decide when a comment is actually clarifying something non-obvious versus just repeating what the code already says? In this particular case, could choosing slightly more descriptive variable or helper names (for example, extracting a small function that classifies paths) remove the need for these comments altogether?

To "like" or "dislike" this comment, please follow this link


result_files = get_visible_entries(result_files, include_hidden)

if len(file_paths) == 1:
entries = list(result_files)
for contents in result_dirs.values():
filtered = get_visible_entries(contents, include_hidden)
entries.extend(filtered)
format_entries(entries, one_per_line)
else:
format_entries(result_files, one_per_line)

for directory, contents in result_dirs.items():
print("\n" + directory + ":")
filtered = get_visible_entries(contents, include_hidden)
format_entries(filtered, one_per_line)
except OSError as err:
print(str(err), file=sys.stderr)

return 0


main()
1 change: 1 addition & 0 deletions implement-shell-tools/wc/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tabulate
92 changes: 92 additions & 0 deletions implement-shell-tools/wc/wc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import argparse
import os
import sys

from tabulate import tabulate


def parse_args():
parser = argparse.ArgumentParser(
description="word, line and byte count",
)
parser.add_argument("paths", nargs="+",
help="The file path(s) to process.")
parser.add_argument(
"-l",
"--lines",
action="store_true",
help="The number of lines in each input file is written to the standard output.",
)
parser.add_argument(
"-w",
"--words",
action="store_true",
help="The number of words in each input file is written to the standard output.",
)
parser.add_argument(
"-c",
"--bytes",
action="store_true",
dest="bytes",
help="The number of bytes in each input file is written to the standard output.",
)
return parser.parse_args()


def main():
args = parse_args()

try:
file_paths: list[str] = args.paths
results: dict[str, dict[str, int]] = {}

for file_path in file_paths:
stats = os.stat(file_path)
count = {"lines": 0, "words": 0, "bytes": stats.st_size}
Comment on lines +44 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In main, the stats variable (line 44) is created only to immediately use stats.st_size when building the count dictionary on the next line. Because stats isn’t reused elsewhere, this temporary variable doesn’t add much clarity and slightly increases the mental overhead when reading the loop.

You might ask yourself: would it be clearer to access the file size directly where it’s needed, or does having a named stats object help you understand the code better? In this particular case, since only the size is used, inlining the os.stat(...).st_size expression into the dictionary could make the code a bit more direct without sacrificing readability.

Thinking along these lines can help you decide when a temporary variable is pulling its weight versus when it’s just an extra name to track.

To "like" or "dislike" this comment, please follow this link


with open(file_path, "r", encoding="utf-8") as file:
for line in file:
count["lines"] += 1
trimmed = line.strip()
if len(trimmed) > 0:
count["words"] += len(trimmed.split())

results[file_path] = count

if len(file_paths) > 1:
total = {"lines": 0, "words": 0, "bytes": 0}
for file_count in results.values():
total["lines"] += file_count["lines"]
total["words"] += file_count["words"]
total["bytes"] += file_count["bytes"]
results["total"] = total

no_options_provided = not (args.lines or args.words or args.bytes)
selected_option_keys: list[str] = []

if args.lines:
selected_option_keys.append("lines")
if args.words:
selected_option_keys.append("words")
if args.bytes:
selected_option_keys.append("bytes")

output_columns = [
"lines", "words", "bytes"] if no_options_provided else selected_option_keys
rows: list[list[str | int]] = []
for name, values in results.items():
rows.append([name] + [values[column] for column in output_columns])

if no_options_provided:
print(tabulate(rows, headers=[
"index"] + output_columns))
else:
print(tabulate(rows, headers=[
"index"] + output_columns))
Comment on lines +74 to +85

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the tabulated output, the first column header is set to "index", but the values you put in that column are actually the file path (or the string "total"), not a numeric index. This can easily mislead someone reading the code or the output into thinking that column contains row numbers.

You might find it useful to ask yourself: if I saw a table with a column called index, what would I expect to be inside it? How could you rename the header so that it more accurately reflects that it holds the file name/path (and total), and keeps the code easier to understand for future readers?

This affects both branches where you call tabulate, since both use the same header list.

To "like" or "dislike" this comment, please follow this link

except OSError as err:
print(str(err), file=sys.stderr)

return 0


main()
Loading