Skip to content
59 changes: 59 additions & 0 deletions implement-shell-tools/cat/cat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import argparse
import sys
from enum import Enum

class Numbering(Enum):
NONE = 0
ALL = 1
NONEMPTY = 2

def print_numbered_line(line, line_number, pad=6):
print(f"{line_number:{pad}}\t{line}", end='')
return line_number + 1

def cat(filepath, numbering, start_line):
line_number = start_line
try:
with open(filepath) as f:
for line in f:
if numbering == Numbering.NONEMPTY:
if line.strip():
line_number = print_numbered_line(line, line_number)
else:
print(line, end='')
elif numbering == Numbering.ALL:
line_number = print_numbered_line(line, line_number)
Comment on lines +19 to +25

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Personally I would probably to make the if more general here and write this as:

Suggested change
if numbering == Numbering.NONEMPTY:
if line.strip():
line_number = print_numbered_line(line, line_number)
else:
print(line, end='')
elif numbering == Numbering.ALL:
line_number = print_numbered_line(line, line_number)
if numbering == Numbering.ALL or (numbering == Numbering.NONEMPTY and line.strip()):
line_number = print_numbered_line(line, line_number)
else:
print(line, end='')

This is because the reason we're picking the two identical behaviours is the same ("We should number this line"), so we ideally only want to write the behaviour once.

And once we've done this, because there's no repetition, I'd probably stop having print_numbered_line return a new number, and instead just:

Suggested change
if numbering == Numbering.NONEMPTY:
if line.strip():
line_number = print_numbered_line(line, line_number)
else:
print(line, end='')
elif numbering == Numbering.ALL:
line_number = print_numbered_line(line, line_number)
if numbering == Numbering.ALL or (numbering == Numbering.NONEMPTY and line.strip()):
print_numbered_line(line, line_number)
line_number += 1
else:
print(line, end='')

else:
print(line, end='')
except FileNotFoundError:
print(
f"cat: {filepath}: No such file or directory",
file=sys.stderr
)
return line_number

def main():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How did you test this implementation?

I created two files and tried using cat -n /file/1 /file/2 and cat -b /file/1 /file/2 and compared the output with using your script, and didn't always get the same results

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment still stands - I get different results between your program and the builtin cat.

parser = argparse.ArgumentParser(
description="Concatenate files and print on the standard output."
)
parser.add_argument('-n', action='store_true', help='number all output lines')
parser.add_argument('-b', action='store_true', help='number non-empty output lines')
parser.add_argument('files', nargs='+', help='files to concatenate')
args = parser.parse_args()

if args.n and args.b:
parser.error("options -n and -b are mutually exclusive")
elif args.n:
numbering = Numbering.ALL
elif args.b:
numbering = Numbering.NONEMPTY
else:
numbering = Numbering.NONE

line_number = 1

for file in args.files:
line_number = cat(file, numbering=numbering, start_line=line_number)

if __name__ == "__main__":
main()
47 changes: 47 additions & 0 deletions implement-shell-tools/ls/ls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import os
import sys
import argparse

def ls(path, one_column, show_hidden):
"""List files in a directory, optionally in one column or including hidden files."""
try:
if os.path.isfile(path):
print(os.path.basename(path))
return

files = os.listdir(path)

if show_hidden:
files = ['.', '..'] + files
else:
files = [f for f in files if not f.startswith('.')]

files.sort()

if one_column:
print(*files, sep='\n')
else:
print(*files)
Comment on lines +21 to +24

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is fine, but an alternative you may want to consider is:

Suggested change
if one_column:
print(*files, sep='\n')
else:
print(*files)
separator = "\t"
if one_column:
separator = "\n"
print(*files, sep=separator)

This may be more or less clear, I'm not going to argue strongly one way or the other, but is in keeping with that general "highlight what's the same and what's different" idea.


except FileNotFoundError:
print(
f"ls: cannot access '{path}': No such file or directory",
file=sys.stderr
)
except NotADirectoryError:
print(
f"ls: cannot access '{path}': Not a directory",
file=sys.stderr
)

def main():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same question about testing here - If I run ls -a I get different files listed than if I run python3 ls.py -a.

parser = argparse.ArgumentParser()
parser.add_argument('-1', dest='one_column', action='store_true', help='list one file per line')
parser.add_argument('-a', action='store_true', help='show hidden files')
parser.add_argument('path', nargs='?', default='.', help='directory to list')
args = parser.parse_args()

ls(args.path, args.one_column, args.a)

if __name__ == "__main__":
main()
87 changes: 87 additions & 0 deletions implement-shell-tools/wc/wc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import argparse
import sys

def wc(path, count_lines, count_words, count_bytes):
"""Count lines, words, and bytes for a single file."""
try:
with open(path, 'r') as f:
content = f.read()

lines = content.splitlines()
words = content.split()

line_count = len(lines)
word_count = len(words)
byte_count = len(content.encode('utf-8'))

if not any([count_lines, count_words, count_bytes]):
count_lines = True
count_words = True
count_bytes = True

parts = []

if count_lines:
parts.append(str(line_count))

if count_words:
parts.append(str(word_count))

if count_bytes:
parts.append(str(byte_count))

print(' '.join(parts), path)

return line_count, word_count, byte_count

except FileNotFoundError:
print(
f"wc: {path}: No such file or directory",
file=sys.stderr
)
return (0, 0, 0)

except IsADirectoryError:
print(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this happens, what will the exit code of the process be? What should it be?

(Same question for FileNotFoundError)

f"wc: {path}: Is a directory",
file=sys.stderr
)
return (0, 0, 0)

def main():
parser = argparse.ArgumentParser()
parser.add_argument('-l', action='store_true', help='Count lines')
parser.add_argument('-w', action='store_true', help='Count words')
parser.add_argument('-c', action='store_true', help='Count bytes')
parser.add_argument('paths', nargs='+', help='Files to count')
args = parser.parse_args()

total_lines = 0
total_words = 0
total_bytes = 0

multiple_files = len(args.paths) > 1
show_all = not any([args.l, args.w, args.c])

for path in args.paths:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I pass multiple files, the real wc outlines a line that you don't - can you add that too?

l, w, b = wc(path, args.l, args.w, args.c)
total_lines += l
total_words += w
total_bytes += b

if multiple_files:
parts = []

if args.l or show_all:
parts.append(str(total_lines))

if args.w or show_all:
parts.append(str(total_words))

if args.c or show_all:
parts.append(str(total_bytes))

print(' '.join(parts), 'total')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is repetitive with the formatting code, but as it's currently structured it's quite fiddly to deduplicate. Here's a set of steps that may help:

  1. Move the "if none of the flags are set, set them all to true" code into just after args = parser.parse_args. Actually update args.l and args.w and args.c as a result. Then you don't need the or show_all any more.
  2. Extract a function def print_stats(args, lines, words, bytes, filename) which you can call from both places.


if __name__ == "__main__":
main()
Loading