I've decided to write some Linux commands in Python. Below is a list, along with some constraints (if you're unfamiliar with Linux, the top of the program along has a description about each command and what it does):
ls
: No constraints, just lists all files/directories in current working directory. No flags.cd
: Can only pass..
or another directory. No flags.tree
: No flags or directories can be passed.clear
: No constraints. No flags.whatis
: Only defined commands can be passed. No flags.cat
: Requires a full path to the file. No flags.
I would like feedback on all the functions embodied by the program below, but whatis
in particular. I feel there is a better way than checking each individual function. Any and all recommendations are appreciated.
"""
This is a program to simulate a command line interface
Commands to reimplement:
- ls ( lists all files and directories in current directory )
- cd [directory] ( changes directory )
- tree ( lists all files, directories, sub directories and files from current directory )
- clear ( clears the console screen )
- whatis [command] ( gives a description about the command )
- cat [file] ( outputs the content of the file )
"""
import os
def ls() -> None:
"""
Lists all files and directories in current directory
"""
current_directory = os.getcwd()
for _, directories, files in os.walk(current_directory):
for file in files:
print(file)
for directory in directories:
print(directory)
def cd(directory: str) -> str:
"""
Changes directory
"""
current_directory = os.getcwd()
if directory == "..":
current_directory = current_directory.split("/")[:-1]
current_directory = ''.join(f"{x}/" for x in current_directory)
return current_directory
if os.path.isdir(directory):
return directory
return "Not a directory"
def tree() -> None:
"""
Lists all files, directories, sub directories and files from current directory
"""
current_directory = os.getcwd()
tab_count = 1
print(current_directory.split("/")[-1])
for _, directories, files in os.walk(current_directory):
for file in files:
print("|" + ("-" * tab_count) + file)
for directory in directories:
print("|" + ("-" * tab_count) + directory)
tab_count += 1
def clear() -> None:
"""
Clears the console screen
"""
print("\n" * 100)
def whatis(command: str) -> None:
"""
Prints a description about the passed command
"""
if command == "ls":
print(ls.__doc__)
elif command == "cd":
print(cd.__doc__)
elif command == "tree":
print(tree.__doc__)
elif command == "clear":
print(clear.__doc__)
elif command == "whatis":
print(whatis.__doc__)
elif command == "cat":
print(cat.__doc__)
else:
print("Not a valid command!")
def cat(file_path: str) -> None:
"""
Accepts a path to a file, and outputs the contents of that file
"""
if os.path.exists(file_path):
with open(file_path, "r") as file:
print(''.join(line for line in file))
else:
print("Not a file!")
2 Answers 2
Here are a few comments. In general, your code deviates in surprising ways from the UNIX commands (and not just by missing flags or options):
Your
ls
command does more than you claim it does.os.walk
recursively "walks" down from the current directory, so it returns the content of all subfolders as well (without indicating in which subfolder each file or directory actually is).When working with paths you should use the library
pathlib
or at leastos.path
consistently. This allows your tools to also work in Windows, where the symbol separating paths is\
and not/
. You could useos.path.join
in yourcd
function. However, all that code there is not needed, becauseos.path.isdir("..")
is alsoTrue
. Useos.path.abspath
to get the absolute name of a path. In my home directoryos.path.abspath("..")
returns/home
.Don't use special return values to indicate failure. What if I want to have a directory called
"Not a directory"
? It may not be the most common name for a directory, but it is at least a plausible name. Instead raise an exception and catch it in the surrounding scope.Note that your
cd
does not actually change the directory, so any calls in another function toos.getcwd()
(like inls
) will still return the old directory.Instead of hard-coding 100 blank lines, try to find out how many lines the terminal has, as taken from this answer by @brokkr:
import os rows, columns = os.popen('stty size', 'r').read().split()
This probably works only on linux, though.
Instead of reading the whole file into memory, joining it and then printing it, you can print it one line at a time. This is slightly slower, but has no memory limitation:
def cat(file_path: str) -> None: """ Accepts a path to a file, and outputs the contents of that file """ if os.path.exists(file_path): with open(file_path, "r") as file: for line in file: print(line, end="") else: print("Not a file!")
As long as all your functions are defined in the same module, you can simplify your
whatis
command:def whatis(command: str) -> None: """ Prints a description about the passed command """ try: print(globals()[command].__doc__) except KeyError: print("Not a valid command!")
Although this does open you up slightly to a user getting access to the
__doc__
attribute of any object in the global namespace (not sure if that is a security risk, though). To avoid this, you can also built a white-listed dictionary:DOCS = {f.__name__: f.__doc__ for f in [ls, cd, clear, tree, whatis, cat]} def whatis(command: str) -> None: """ Prints a description about the passed command """ try: print(DOCS[command]) except KeyError: print("Not a valid command!")
I think the docstrings should be in the same style as linux manpages, i.e. like you have in your module docstring.
-
\$\begingroup\$ You could say that the OP's code implements
ls -R
without the directory-name headers that actualls
prints when recursing. Or more exactly:find -printf '%f\n'
instead ofls
. \$\endgroup\$Peter Cordes– Peter Cordes2019年10月11日 17:45:44 +00:00Commented Oct 11, 2019 at 17:45 -
\$\begingroup\$ @PeterCordes True. But then this should be reflected in the docstring. \$\endgroup\$Graipher– Graipher2019年10月11日 17:46:54 +00:00Commented Oct 11, 2019 at 17:46
-
1\$\begingroup\$ Oh yeah, I assume it was unintentional, and certainly not very useful. Just maybe fun to put it in those terms to illustrate to the OP what the symptom of their bug is, and what command they actually implemented by mistake. (Maybe they do want to implement
find
next...) \$\endgroup\$Peter Cordes– Peter Cordes2019年10月11日 17:49:09 +00:00Commented Oct 11, 2019 at 17:49
There are a few ways to programmatically get the docs, and one that I find cool to use is inspect
.
inspect
allows you, among other things, to check what's in your file (module):
import inspect
import sys
docs = {} # a dictionary
module = sys.modules[__name__] # Gets us a reference to the current module
for name, object in inspect.getmembers(module):
# getmembers returns the name of the object, then the object itself.
if inspect.isfunction(func): # Filter the functions
docs[name] = object.__doc__
This can be rewritten as a dictionary list comprehension:
module = sys.modules[__name__]
docs = {name: func.__doc__ for name, obj in inspect.getmembers(module)
if inspect.isfunction(obj)}
Then, this could be defined as a constant:
module = sys.modules[__name__]
DOCS = {name: func.__doc__ for name, obj in inspect.getmembers(module)
if inspect.isfunction(obj)}
def whatis(command: str) -> None:
print(DOCS.get(command, "Not a valid command!"))
dict.get
allows to specify a default value if the key is not in the dict (here, the error message).
This shows an alternative to your implementation, but I would not use it myself, for it's not so clear at first glance. What I like about it, though, is to have a constant dictionary, for this module is concise enough:
DOCS = {
'ls' = ls.__doc__,
'cd' = cd.__doc__,
'tree' = tree.__doc__,
'clear' = clear.__doc__,
'whatis' = whatis.__doc__,
'cat' = cat.__doc__,
}
-
2\$\begingroup\$ You already use
inspect
, so it might be worth to have a look atinspect.getdoc(...)
. \$\endgroup\$AlexV– AlexV2019年10月11日 11:45:16 +00:00Commented Oct 11, 2019 at 11:45
Explore related questions
See similar questions with these tags.
ls
is in theos
module, you can useos.listdir()
as well ascd
for which you can useod.chdir(path)
and forclear
you may useos.system('cls')
\$\endgroup\$clear
instead ofcls
. \$\endgroup\$cls
is for windows systems, andclear
is for unix systems \$\endgroup\$