4
\$\begingroup\$

I've refactored the previous version of my local source control, and revised a few things. It's mostly the same, but there are a few minor differences, like the argument separator, one new command, and (hopefully) cleaner code.


How does it work?

Each "project" written with this has a specific file structure that looks like the below. A file structure like this is required in order for the source control to work.

/[Project Name]
    /source
        ...

    /versions
        ...

    /info
        changelog.txt
        readme.txt

There are also three commands that are used. new_project, push_version, change_dir, and exit_shell. Each command argument is comma separated, and look something like the following:

  • new_project, [project Name], [project info] - Create a new project.
  • push_version, [version name], [version info] - Push a new version to the versions folder as a .zip file.
  • change_dir, [directory] - Change to a new directory.
  • exit_shell, [status] - Exit the shell.

Concerns

There are a few things that I'm concerned about here.

  • Am I correctly handling errors?
  • Is there a better way to check for valid arguments length, rather than writing the same block of code?
  • Is this a good way to "parse" input? Should I be using an actual parsing library rather than just using regular expressions and string splitting?

lsc.py

import re
import os

import commands


def execute_user_input(tokenized_user_input: list):
    """Execute tokenized user input.

    This function executes user input, like the
    command "new::Awesome Project::A really awesome project."
    after it's been tokenized.

    Keyword arguments:
    tokenized_user_input -- The tokenized form of the user's input.
    """
    COMMANDS = {
        "new_project": commands.new_project,
        "push_version": commands.push_version,
        "change_dir": commands.change_directory,
        "exit_shell": commands.exit_shell
    }

    command = tokenized_user_input[0]
    arguments = tokenized_user_input[1:]

    if command in COMMANDS:
        COMMANDS[command](arguments)
    else:
        print("Invalid command \"{0}\".".format(command))


def tokenize_user_input(user_input):
    """Tokenize user input into a list.

    This function tokenizes the user's input into a
    form that the execute_user_input function can
    correctly interpret.

    Keyword arguments:
    user_input -- The user's input.
    """
    return re.split("\s*,\s*", user_input)


def main():
    while True:
        user_input = input("{0} $ ".format(os.getcwd()))
        execute_user_input(tokenize_user_input(user_input))


if __name__ == "__main__":
    main()

commands.py

import os
import re
import sys
import shutil


def new_project(arguments):
    """Generate a new project. (`new_project`)

    This function generates a new project. A
    project structure looks like this:

        /[project name]
            /source
                ...

            /versions
                ...

            /info
                readme.txt
                changelog.txt

    Keyword arguments:
    project_name        -- The name of the project.
    project_description -- A brief description of the project.
    """
    if len(arguments) != 2:
        print("Invalid arguments length of \"{0}\".".format(len(arguments)))
        return

    project_name = arguments[0]
    project_description = arguments[1]

    os.makedirs("./{0}".format(project_name))
    os.chdir("./{0}".format(project_name))

    os.makedirs("./source")
    os.makedirs("./versions")
    os.makedirs("./info")

    with \
        open("./info/readme.txt", "w+") as readme, \
        open("./info/changelog.txt", "w+") as changelog:
        readme.write(project_description)


def push_version(arguments):
    """Push a new version. (`push_version`)

    This function pushes a the contents of the
    ./[project name]/source folder to a new folder
    in ./[project name]/versions named with the
    version number.

    Keyword arguments:
    version_number      -- The version number. Must contain valid characters for folder names.
    version_description -- A brief description of the changes in the version to be written to the changelog.
    """
    if len(arguments) != 2:
        print("Invalid arguments length of \"{0}\".".format(len(arguments)))
        return

    version_number = arguments[0]
    version_description = arguments[1]

    if re.match("[a-zA-Z0-9_\-\s\.]+", version_number):
        with open("./info/changelog.txt", "a") as changelog:
            shutil.make_archive("./versions/{0}".format(version_number), format="zip", root_dir="./source")
            changelog.write("=== Version: {0} ===".format(version_number))
            changelog.write(version_description + "\n")
    else:
        print("Version number does not match the regular expression \"[a-zA-Z0-9_\-\s\.]+\".")


def change_directory(arguments):
    """Change to a new directory. (`change_dir`)

    This function allows the user to navigate to
    another directory.

    Keyword arguments:
    directory -- The directory to navigate to.
    """
    if len(arguments) != 1:
        print("Invalid arguments length of \"{0}\".".format(len(arguments)))
        return

    directory = arguments[0]

    try:
        os.chdir(directory)
    except FileNotFoundError:
        print("Invalid directory \"{0}\"".format(directory))


def exit_shell(arguments):
    """Exit the LSC shell.

    This function exits the shell, along
    with an additional status argument.

    Keyword arguments:
    status -- The status to exit with.
    """
    status = arguments[0]
    exit(status)

If you're curious about this project, you can find it on Github, here.

\$\endgroup\$
1
\$\begingroup\$

Command argument validation

As you suspected, the code validating arguments in each command is tedious. Even worse that the method signatures are not useful because of the current method.

You can do much better, using the inspect package. Instead of validating the argument count in each command method, you can do it in execute_user_input before calling them.

First, change the method signatures to the way you would write naturally in a Python program, for example:

def change_directory(directory):
    # ...

def push_version(version_number, version_description):
    # ...

And of course, delete the validation logic from these methods.

Next, add the validation logic using inspect in the execute_user_input function:

argspec = inspect.getargspec(COMMANDS[command])
expected = len(argspec.args)
actual = len(arguments)
if expected != actual:
    print('Wrong number of arguments: {} != {}'.format(actual, expected))
else:
    COMMANDS[command](arguments)

With this change, the entire code will become more compact, the command functions will become more natural, and documentable using standard doc strings.

Error handling

The biggest issues I see with error handling:

  • Not considering some corner cases
  • Not useful messages

For example here:

try:
    os.chdir(directory)
except FileNotFoundError:
    print("Invalid directory \"{0}\"".format(directory))

FileNotFoundError is but one of the things that can go wrong. Other things that can go wrong:

  • directory is actually a file, raising NotADirectoryError
  • directory is not accessible due to permissions, raising PermissionError

And the error message is not helpful: what does it mean, "Invalid directory" ? The natural message would have been "No such directory:".

Usability

The command names are too long. For example, instead of change_dir, simply cd would be natural, and welcoming to fans of the shell.

The tokenizer requires at least a command + space between arguments. As a lazy typist, that's annoying.

Coding style

PEP8 recommends using spaces for indentation. I was very surprised to see tab-indented code in your repo.

In addition:

  • Local variables should be lowercase, violated by COMMANDS, which would be better at package scope, rather than a local var.
  • commands.py imports sys but doesn't use it
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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