9
\$\begingroup\$

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!")
\$\endgroup\$
7
  • \$\begingroup\$ ls is in the os module, you can use os.listdir() as well as cd for which you can use od.chdir(path) and for clear you may use os.system('cls') \$\endgroup\$
    – watch-this
    Oct 11, 2019 at 5:29
  • 2
    \$\begingroup\$ @bullseye I realize those are available commands, but I tried to use the least amount of built in functions as I could. \$\endgroup\$
    – Linny
    Oct 11, 2019 at 5:34
  • 1
    \$\begingroup\$ @bullseye I think you mean clear instead of cls. \$\endgroup\$
    – marcelm
    Oct 11, 2019 at 16:05
  • \$\begingroup\$ These commands look like meant for user interactive session. If that is the case, and you are not implementing them just as an exercise, I highly recommend using some more advanced python shell instead, such as ipython (available both for Python-2 and python-3, both for Linux and Windows, possible to install via pip). It features history access and search, tab completion of commands AND filenames, AND you can execute any actual linux shell command by simply prepending it with "!" \$\endgroup\$
    – Gnudiff
    Oct 11, 2019 at 18:40
  • \$\begingroup\$ @bullseye You have those two confused. cls is for windows systems, and clear is for unix systems \$\endgroup\$
    – Linny
    Oct 11, 2019 at 20:35

2 Answers 2

17
\$\begingroup\$

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 least os.path consistently. This allows your tools to also work in Windows, where the symbol separating paths is \ and not /. You could use os.path.join in your cd function. However, all that code there is not needed, because os.path.isdir("..") is also True. Use os.path.abspath to get the absolute name of a path. In my home directory os.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 to os.getcwd() (like in ls) 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.

\$\endgroup\$
3
  • \$\begingroup\$ You could say that the OP's code implements ls -R without the directory-name headers that actual ls prints when recursing. Or more exactly: find -printf '%f\n' instead of ls. \$\endgroup\$ Oct 11, 2019 at 17:45
  • \$\begingroup\$ @PeterCordes True. But then this should be reflected in the docstring. \$\endgroup\$
    – Graipher
    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\$ Oct 11, 2019 at 17:49
6
\$\begingroup\$

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__,
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ You already use inspect, so it might be worth to have a look at inspect.getdoc(...). \$\endgroup\$
    – AlexV
    Oct 11, 2019 at 11:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.