Python-based Git pre-commit hook to manage multiple users/Git identities

Question

A couple of months ago I posted a bash script to manage multiple Git identities as a solution on Stack Overflow but soon found the hook isn't flexible enough.

Thus I decided to rewrite the hook in Python. Basically it does what it should but I'd really like to have someone review the code in order to eliminate bad practices, to remove unnecessary complexity and how it could be improved in regards to style, readability and performance.

Another question is if you see any point where it would be better to introduce a class.

For the rewrite I created a git repository at github.com: git-passport - A Git command and hook written in Python to manage multiple Git accounts / user identities.

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
""" git-passport is a Git pre-commit hook written in Python to manage
 multiple Git user identities.
"""
# ..................................................................... Imports
import configparser
import os.path
import subprocess
import sys
import textwrap
import time
import urllib.parse
# ............................................................ Config functions
def config_create(filename):
 """ Create a configuration file containing sample data inside the home
 directory if none exists yet.
 Args:
 filename (str): The complete `filepath` of the configuration file
 """
 if not os.path.exists(filename):
 preset = configparser.ConfigParser()
 preset["General"] = {}
 preset["General"]["enable_hook"] = "True"
 preset["General"]["sleep_duration"] = "0.75"
 preset["Passport 0"] = {}
 preset["Passport 0"]["email"] = "[email protected]"
 preset["Passport 0"]["name"] = "name_0"
 preset["Passport 0"]["service"] = "github.com"
 preset["Passport 1"] = {}
 preset["Passport 1"]["email"] = "[email protected]"
 preset["Passport 1"]["name"] = "name_1"
 preset["Passport 1"]["service"] = "gitlab.com"
 try:
 msg = """
 No configuration file found.
 Generating a sample configuration file.
 """
 print(textwrap.dedent(msg).strip())
 with open(filename, "w") as configfile:
 preset.write(configfile)
 sys.exit("\n~Done~")
 except Exception as error:
 print(error)
 raise sys.exit("\n~Quitting~")
def config_read(filename):
 """ Read a provided configuration file and «import» allowed sections and
 their keys/values into a dictionary.
 Args:
 filename (str): The complete `filepath` of the configuration file
 Returns:
 config (dict): Contains all allowed configuration sections
 """
 raw_config = configparser.ConfigParser()
 raw_config.read(filename)
 # Match an arbitrary number of sections starting with pattern
 pattern = "Passport"
 # A generator to filter matching sections:
 # Let's see if user defined config sections match a pattern
 def generate_matches():
 for section in raw_config.items():
 if pattern in section[0]:
 yield dict(section[1])
 # Construct a custom dict containing allowed sections
 config = dict(raw_config.items("General"))
 config["git_local_ids"] = dict(enumerate(generate_matches()))
 return config
def config_validate(config):
 """ Validate and convert certain keys and values of a given dictionary
 containing a set of configuration options. If unexpected values are
 found we quit the script and notify the user what went wrong.
 Since ``ConfigParser`` only accepts strings when setting up a default
 config it is necessary to convert some values to numbers and boolean.
 Args:
 config (dict): Contains all allowed configuration sections
 Returns:
 config (dict): Contains valid and converted configuration options
 """
 for key, value in config.items():
 if key == "enable_hook":
 if value == "True":
 config[key] = True
 elif value == "False":
 config[key] = False
 else:
 msg = "E > Settings > {}: Expecting True or False."
 raise sys.exit(msg).format(key)
 elif key == "sleep_duration":
 try:
 config[key] = float(value)
 except ValueError:
 msg = "E > Settings > {}: Expecting float or number."
 raise sys.exit(msg).format(key)
 # Here the values could really be anything...
 elif key == "git_local_ids":
 pass
 else:
 msg = "E > Settings > {}: Section/key unknown."
 raise sys.exit(msg).format(key)
 return config
# ............................................................... Git functions
def git_get_id(config, scope, property):
 """ Get the email address or username of the global or local Git ID.
 Args:
 config (dict): Contains validated configuration options
 scope (str): Search inside a `global` or `local` scope
 property (str): Type of `email` or `name`
 Returns:
 git_id (str): A name or email address
 error (str): Exception
 """
 try:
 git_process = subprocess.Popen([
 "git",
 "config",
 "--get",
 "--" + scope,
 "user." + property
 ], stdout=subprocess.PIPE)
 git_id = git_process.communicate()[0].decode("utf-8")
 return git_id.replace("\n", "")
 except Exception as error:
 raise error
def git_get_url():
 """ Get the local remote.origin.url of a Git repository.
 Returns:
 git_url (str): The local and active remote.origin.url
 error (str): Exception
 """
 try:
 git_process = subprocess.Popen([
 "git",
 "config",
 "--get",
 "--local",
 "remote.origin.url"
 ], stdout=subprocess.PIPE)
 git_url = git_process.communicate()[0].decode("utf-8")
 return git_url.replace("\n", "")
 except Exception as error:
 raise error
def git_set_id(config, value, property):
 """ Set the email address or username as a local Git ID for a repository.
 Args:
 config (dict): Contains validated configuration options
 value (str): A name or email address
 property (str): Type of `email` or `name`
 Returns:
 error (str): Exception
 """
 try:
 subprocess.Popen([
 "git",
 "config",
 "--local",
 "user." + property,
 value
 ], stdout=subprocess.PIPE)
 except Exception as error:
 raise error
# ............................................................ Helper functions
def get_user_input(pool):
 """ Prompt a user to select a number from a list of numbers representing
 available Git IDs. Optionally the user can choose `q` to quit the
 selection process.
 Args:
 pool (list): A list of numbers representing available Git IDs
 Returns:
 selection (int): A number representing a Git ID chosen by a user
 """
 while True:
 # https://stackoverflow.com/questions/7437261/how-is-it-possible-to-use-raw-input-in-a-python-git-hook
 sys.stdin = open("/dev/tty")
 selection = input("» Select a passport [ID] or «(q)uit»: ")
 try:
 selection = int(selection)
 except ValueError:
 if selection == "q" or selection == "quit":
 sys.exit("\n~Quitting~\n")
 continue
 if selection not in pool:
 continue
 break
 return selection
def print_choice(choice):
 """ Before showing the actual prompt by calling `get_user_input()` print a
 list of available Git IDs containing properties ID, «scope», name,
 email and service.
 Args:
 choice (dict): Contains a list of preselected Git ID candidates
 """
 for key, value in choice.items():
 if value.get("flag") == "global":
 msg = """
 ~:Global ID: {}
 . User: {}
 . E-Mail: {}
 """
 print(textwrap.dedent(msg).lstrip().format(
 key,
 value["name"],
 value["email"])
 )
 else:
 msg = """
 ~Passport ID: {}
 . User: {}
 . E-Mail: {}
 . Service: {}
 """
 print(textwrap.dedent(msg).lstrip().format(
 key,
 value["name"],
 value["email"],
 value["service"])
 )
def add_global_id(config, target):
 """ Adds the global Git ID to a dictionary containing potential preselected
 candidates.
 Args:
 config (dict): Contains validated configuration options
 target (dict): Contains preselected local Git IDs
 """
 global_email = git_get_id(config, "global", "email")
 global_name = git_get_id(config, "global", "name")
 local_ids = config["git_local_ids"]
 if global_email and global_name:
 position = len(local_ids)
 target[position] = {}
 target[position]["email"] = global_email
 target[position]["name"] = global_name
 target[position]["flag"] = "global"
# .............................................................. Implementation
def identity_exists(config, email, name, url):
 """ Prints an existing ID of a local gitconfig.
 Args:
 config (dict): Contains validated configuration options
 email (str): An email address
 name (str): A name
 url (str): A remote.origin.url
 """
 duration = config["sleep_duration"]
 if not url:
 url = "«remote.origin.url» is not set."
 msg = """
 ~Intermission~
 ~Active Passport:
 . User: {}
 . E-Mail: {}
 . Remote: {}
 """
 print(textwrap.dedent(msg).lstrip().format(name, email, url))
 sys.exit(time.sleep(duration))
def url_exists(config, url):
 """ If a local gitconfig contains a remote.origin.url add all user defined
 Git IDs matching remote.origin.url as a candidate. However if there is
 not a single match then add all available user defined Git IDs and the
 global Git ID as candidates.
 Args:
 config (dict): Contains validated configuration options
 url (str): A remote.origin.url
 Returns:
 candidates (dict): Contains preselected Git ID candidates
 """
 local_ids = config["git_local_ids"]
 netloc = urllib.parse.urlparse(url)[1]
 # A generator to filter matching sections:
 # Let's see if user defined IDs match remote.origin.url
 def generate_candidates():
 for key, value in local_ids.items():
 if value.get("service") == netloc:
 yield (key, value)
 candidates = dict(generate_candidates())
 if len(candidates) >= 1:
 msg = """
 ~Intermission~
 One or more identities match your current git provider.
 remote.origin.url: {}
 """
 print(textwrap.dedent(msg).lstrip().format(url))
 else:
 candidates = local_ids
 msg = """
 ~Intermission~
 Zero passports matching - listing all passports.
 remote.origin.url: {}
 """
 print(textwrap.dedent(msg).lstrip().format(url))
 add_global_id(config, candidates)
 print_choice(candidates)
 return candidates
def no_url_exists(config, url):
 """ If a local gitconfig does not contain a remote.origin.url add
 all available user defined Git IDs and the global Git ID as
 candidates.
 Args:
 config (dict): Contains validated configuration options
 url (str): A remote.origin.url
 Returns:
 candidates (dict): Contains preselected Git ID candidates
 """
 candidates = config["git_local_ids"]
 msg = """
 ~Intermission~
 «remote.origin.url» is not set, listing all IDs:
 """
 add_global_id(config, candidates)
 print(textwrap.dedent(msg).lstrip())
 print_choice(candidates)
 return candidates
# ........................................................................ Glue
def main():
 config_file = os.path.expanduser("~/.git_passport")
 config_create(config_file)
 config = config_validate(config_read(config_file))
 if config["enable_hook"]:
 local_email = git_get_id(config, "local", "email")
 local_name = git_get_id(config, "local", "name")
 local_url = git_get_url()
 if local_email and local_name:
 identity_exists(config, local_email, local_name, local_url)
 elif local_url:
 candidates = url_exists(config, local_url)
 else:
 candidates = no_url_exists(config, local_url)
 selected_id = get_user_input(candidates.keys())
 git_set_id(config, candidates[selected_id]["email"], "email")
 git_set_id(config, candidates[selected_id]["name"], "name")
 print("\n~Done~\n")
if __name__ == "__main__":
 main()

EDIT: Since there has been posted an answer I'll leave the question as it is / freeze it in order to avoid any discrepancies. Development continues at the repository.

Answer 1

Architecture / general thoughts

Overall looks good. You have lots of mostly useful comments which help to understand the program, the code is clear if a little verbose, so to me this was easy to read.

That said I'm going to list a few things which could improve the program and then continue with in detail code issues below.

Hope that helps and good luck with the program, it looks like a useful tool e.g. if you work both on personal projects and e.g. for a company. Could you post a link to your repository? It would be great if you could package and release this. A quick search also mentions the need to use different Github user names and SSH keys, so there are definitely more use cases (and options) to consider in the future.

• I'd get rid of the config_validate function and do that in config_read instead, using a separate config_read_passport or so, which then uses ConfigParser.getboolean and the other get... functions with a very strict schema, so you make sure that everything coming out of the config object is properly parsed. That way you have to write less verification code (as the ConfigParser object takes care of that for you) and you can spend that on making sure that the individual passports have the correct format before handing them off to the rest of the application.
• I personally don't use origin as a remote very often. It would be great if that could be configurable.
• A script like this is probably fine without using classes. I can see how using it for the configuration would help structuring it better, but it's by no means necessary if this works for you.
• I kind of find the output with lots of irregular characters unexpected, but of course that's your choice.

Code

• The docstrings say Returns: error (str): Exception, but the functions don't return exceptions, they raise them, so the docstring should say Throws or Raises instead and mention what kind of exception it uses. If you can't know that, e.g. in most of the git_* functions, leave it out, or refer to the specific function which might cause problems, i.e. subprocess.Popen.
• Just as example, in config_create you can remove one level of indentation if you return early, i.e. if not os.path.exists(filename): return;.
• This is a style choice, but the creation of the preset dictionary can be shorter if you'd just use the literal dictionary syntax, i.e. preset = {"General": {...}, "Passport 0": {...}, ...} instead of repeating the keys all the time.
• textwrap.dedent(foo).strip() is nice, I'm copying that; since you use it very often I think that separate functions, like dedented or so, are in order; something short and simple. Same for lstrip.
• sys.exit already exits the process, no raise necessary, unless I'm completely missing the idiom here. And then maybe don't catch the exception in the first place, just let it propagate. The result will be the same if you already print the exception.
• Also, I would use less sys.exits in general. It is helpful if you can just import the script for testing purposes and it's jarring if using e.g. config_create suddenly kills the interpreter. Same for proper testing later.
• generate_matches could very well be a regular function and accept the two arguments pattern and raw_config instead.
• I'd wrap getting values from git in a separate function, maybe git_config_get, which would (for now) still use the same subprocess.Popen method with passed in arguments and then does the communicate/decode handling. And reraising the exception isn't necessary. Whether you create a separate git_config_set (instead of using the same mechanism as git config) is kind of a trade-off. Performance-wise there's always the option to not call a separate program and use something like a libgit binding, e.g. pygit2 instead.
• get_user_input should reset sys.stdin to its previous value I think. Again, think of reusing this; same with the sys.exit.
• I'd structure the loop more like while True: read selection; if in pool: return selection. That's way less confusing than if not in pool: continue; (else) break; return.
• For performance, you can always use iteritems on dictionaries if you don't need the intermediate list.
• add_global_id does nothing if either global_email or global_name doesn't exist. Shouldn't it rather mention that problem to the user?
• sys.exit(time.sleep(..)) implies that the return value of time.sleep is somehow significant. time.sleep(); sys.exit() is clearer. I'd also consider waiting for a keypress (or newline) from the user before exiting instead of using a timeout.

• Saucier: Thanks a lot for your feedback I'll work through your review step by step during the next week. I updated the question with a link to the repository.