At least the code is not at the top-level of the file, but you should really use functions to document what is going on. Well chosen names can be a long way into conveying what you are doing and why.
I also have a tendency to extract out the argparse
machinery out of the way of the actual computation. This way, I can mock this part in my tests:
>>> class Args:
... def __getattr__(self, key):
... return None
...
>>> args = Args()
>>> args.foo
>>> args.spam = 'egg'
>>> args.bacon = 'foo'
>>> vars(args)
{'spam': 'egg', 'bacon': 'foo'}
and call main(args)
(or in your case cat(args)
) without having to rely on the command-line. So lets build that:
def parse_command_line():
parser = argparse.ArgumentParser(
description='concatenate files and print on the standard output')
parser.add_argument('filenames', nargs='+', help='directory to scan')
parser.add_argument('-r', '--repeat', dest='repeat', default=1, type=int,
help='repeat output multiple times')
parser.add_argument('-f', '--header', dest='header', default=False,
help='precede every file with its name',
action='store_true')
parser.add_argument('-n', '--number', dest='number', default=False,
help='number all output lines', action='store_true')
parser.add_argument('-b', '--number-nonblank', dest='number_nonblank',
default=False, help='number nonempty output lines',
action='store_true')
parser.add_argument('-A', '--show-all', dest='show_all', default=False,
help='equivalent to -vET', action='store_true')
parser.add_argument('-e', dest='show_ends_and_nonprinting', default=False,
help='equivalent to -vE', action='store_true')
parser.add_argument('-E', '--show-ends', dest='show_ends', default=False,
help='display $ at end of each line', action='store_true')
parser.add_argument('-t', dest='show_tabs_and_nonprinting', default=False,
help='equivalent to -vT', action='store_true')
parser.add_argument('-T', '--show-tabs', dest='show_tabs', default=False,
help='display TAB characters as ^I', action='store_true')
parser.add_argument('-v', '--show-nonprinting', dest='show_nonprinting',
default=False, action='store_true',
help='use ^ and M- notation, except for LFD and TAB')
args = parser.parse_args()
# Special chars option overlapping
if args.show_all:
args.show_tabs = True
args.show_ends = True
args.show_nonprinting = True
if args.show_tabs_and_nonprinting:
args.show_tabs = True
args.show_nonprinting = True
if args.show_ends_and_nonprinting:
args.show_ends = True
args.show_nonprinting = True
if args.number_nonblank:
args.number = True # See later why
return args
I also inverted a bit the management of overlapping options as I feel it reads better.
This way, even if you put the rest of the code into a single def cat(args):
function, the main part is down to:
if __name__ == '__main__':
args = parse_command_line()
cat(args)
which should be much easier to start testing.
Now, as regard to the core of your program, I can see at least 2 undocumented behaviour that differ from the cat
utility on linux. First of, you’re only ever reading one line of input instead of the whole stdin
. Consider use cases like grep PATTERN unfiltered.txt | cat header.txt - footer.txt
that the original cat
handles perfectly (i.e. all of the output of grep
is printed between the content of header.txt
and footer.txt
). You don't. Since I also don't understand why you would repeat every other files but stdin
, I suggest using the tempfile
module to buffer the content of stdin
.
We will thus need to change the content of the input array, so let's encapsulate that in a function that perform the globing too, to clean up this chain
command. While I’m at it, cat
also allow to specify no filename on the command line, which is treated the same as a single '-'
so let's fix that as well:
def sanitize_filenames(file_patterns):
for pattern in file_patterns:
if pattern == '-':
yield buffer_stdin()
else:
yield from glob.glob(pattern)
if not file_patterns:
yield buffer_stdin()
def buffer_stdin():
with tempfile.NamedTemporaryFile(delete=False) as stdin:
for line in sys.stdin:
stdin.write(line)
return stdin.name
Usage being: filenames = list(sanitize_filenames(args.filenames))
. However, this implementation leave a file in the temporary directory for each '-'
present on the command line; I’ll get to it latter.
The second behaviour that differs is the way you handle line counts on -n
or -b
options. cat
uses the equivalent of '{:>6}\t'.format(line_number)
to append them when needed; making the alignment between line 999999 and 1000000 go:
999999 one line
1000000 another line
and the one between line 9999999 and 10000000 go:
9999999 one line
10000000 another line
If you made the same, you could simplify the code a bit by not having to read each file upfront.
You also handle -Eb
a "better" way than cat
which just remove the formatting altogether:
9999999 one line$
$
10000000 another line$
I’ll try to keep that. I would however change the way you handle nonprinting characters because the special cases I
and J
are in fact handling of tabs and newlines. So I'd use a single translate
instead of replace
+ translate
.
def prepend_line_numbers(numbers):
print('{:>6}\t'.format(numbers), end='')
def build_special_chars_table(args):
table = {
9: '^I' if args.show_tabs else '\t',
10: '$' if args.show_ends else '',
}
if args.show_nonprinting:
nonprinting_representation = '@{}[\]^!'.format(string.ascii_uppercase)
table.update({
k: '^' + v
for k, v in enumerate(non_printing_representation)
if v not in 'IJ'
})
return table
def cat(args):
nonprinting = build_special_chars_table(args)
header_width = shutil.get_terminal_size().columns // 2
line_count = 0
for filename in sanitize_filenames(args.filenames):
if args.header:
print('{:=^{}}'.format(filename, header_width))
for _ in range(args.repeat):
with open(filename, 'rb') as file:
for line in file:
blank_line = not line.rstrip()
if not (args.number_nonblank and blank_line):
line_count += 1
if args.number:
prepend_line_numbers(line_count)
elif args.number: # Useful there to have that True even when -b only
# For -E under -b
prepend_line_numbers(' ' * len(str(line_count)))
print(line
.decode(errors='backslashreplace')
.translate(nonprinting))
Now to get back on the files staying on disc after that utily ran over some '-'
on the command line, you can define a class like:
class StdInWrapper:
def __init__(self):
with tempfile.NamedTemporaryFile(delete=False) as stdin:
self.name = stdin.name
for line in sys.stdin:
stdin.write(line)
def __str__(self):
return self.name
def remove(self):
os.remove(self.name)
instead of the buffer_stdin
function. You would need to change the call to open(filename, 'rb')
into open(str(filename), 'rb')
so that it still work and figure a way to call remove on that object at the end of the repetitions.