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
5 changes: 5 additions & 0 deletions implement-shell-tools/cat/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ It must act the same as `cat` would, if run from the directory containing this R
Matching any additional behaviours or flags are optional stretch goals.

We recommend you start off supporting no flags, then add support for `-n`, then add support for `-b`.

Run the Python implementation from this directory with `python cat.py sample-files/1.txt`
or `python cat.py -n sample-files/1.txt sample-files/2.txt`. Use `-b` to number
nonempty lines. Python 3 is required. Pass filenames explicitly if your shell
does not expand wildcards.
34 changes: 34 additions & 0 deletions implement-shell-tools/cat/cat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
"""Concatenate files, optionally numbering their lines."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The module docstring on line 1 explains what the script does at a high level. In many small scripts, the combination of the filename (cat.py), the function/argument names, and the argument help texts already make the intent very clear. In those cases, an extra docstring can end up repeating information without adding much new context.

It might be worth asking yourself: Does this comment tell a future reader anything they couldn’t already infer from the code and CLI help? If the answer is no, you could either drop the docstring or expand it to include non-obvious behavior (for example, how -n and -b interact, or how errors are handled). That way, any comment you keep is clearly pulling its weight.

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


import argparse
from pathlib import Path
import sys


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("-n", action="store_true", help="number every line")
parser.add_argument("-b", action="store_true", help="number nonempty lines")
parser.add_argument("files", nargs="+")
args = parser.parse_args()
status = 0

for filename in args.files:
line_number = 1
try:
with Path(filename).open(encoding="utf-8", newline="") as source:
for line in source:
number_line = args.n or (args.b and bool(line.strip()))
if number_line:
sys.stdout.write(f"{line_number:6}\t")
line_number += 1
Comment on lines +16 to +24

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 your current implementation, line_number is defined and initialized inside the for filename in args.files: loop. That means line_number is reset to 1 for each file, so numbering starts over for every new file in a multi-file invocation.

On both common cat implementations (GNU on Linux and BSD on macOS), when you use -n or -b with multiple files, the line numbers continue increasing across files rather than restarting at 1 for each file. Since your README says this tool should act the same as cat when run from this directory, resetting line_number per file will give different output from the real cat in multi-file cases.

Would you consider where line_number needs to live so that it can persist across the whole run instead of being recreated for each file? Moving it to a scope that spans all files would let numbering continue correctly across files while still letting you increment it inside the inner loop.

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

sys.stdout.write(line)
except (OSError, UnicodeError) as error:
print(f"cat: {filename}: {error}", file=sys.stderr)
status = 1

return status


if __name__ == "__main__":
sys.exit(main())
Loading