Merge pull request #261 from jakub-nt/_minor-code-improvements-2

Code quality improvements

Author: olehermanse

cfbs/args.py

@@ -20,7 +20,7 @@ class ArgsTypesNamespace(argparse.Namespace):
     check: bool
     checksum: Union[str, None]
     keep_order: bool
-    git: Union[str, None]
+    git: Union[bool, None]
     git_user_name: Union[str, None]
     git_user_email: Union[str, None]
     git_commit_message: Union[str, None]
@@ -63,6 +63,21 @@ def get_manual():
         raise CFBSExitError("Manual file does not exist")
 
 
+def yesno_to_bool(s: str):
+    if s == "yes":
+        return True
+    if s == "no":
+        return False
+    assert False
+
+
+class YesNoToBool(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        assert type(values) is str
+        values = yesno_to_bool(values)
+        setattr(namespace, self.dest, values)
+
+
 @cache
 def get_arg_parser():
     command_list = commands.get_command_names()
@@ -116,6 +131,7 @@ def get_arg_parser():
     parser.add_argument(
         "--git",
         choices=("yes", "no"),
+        action=YesNoToBool,
         help="Override git option in cfbs.json",
     )
     parser.add_argument(

cfbs/commands.py

@@ -100,11 +100,8 @@ def search_command(terms: List[str]) -> int:
 )
 from cfbs.index import _VERSION_INDEX, Index
 from cfbs.git import (
-    git_exists,
+    git_configure_and_initialize,
     is_git_repo,
-    git_get_config,
-    git_set_config,
-    git_init,
     CFBSGitError,
     ls_remote,
 )
@@ -169,11 +166,16 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int:
 
 
 @cfbs_command("init")
-def init_command(index=None, masterfiles=None, non_interactive=False) -> int:
+def init_command(
+    index=None,
+    masterfiles=None,
+    non_interactive=False,
+    use_git: Union[bool, None] = None,
+) -> int:
     if is_cfbs_repo():
         raise CFBSUserError("Already initialized - look at %s" % cfbs_filename())
 
-    name = prompt_user(
+    project_name = prompt_user(
         non_interactive,
         "Please enter the name of this CFEngine Build project",
         default="Example project",
@@ -186,7 +188,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int:
 
     config = OrderedDict(
         {
-            "name": name,
+            "name": project_name,
             "type": "policy-set",  # TODO: Prompt whether user wants to make a module
             "description": description,
             "build": [],
@@ -195,81 +197,47 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int:
     if index:
         config["index"] = index
 
-    do_git = get_args().git
-    is_git = is_git_repo()
-    if do_git is None:
-        if is_git:
-            do_git = prompt_user_yesno(
+    if use_git is None:
+        if is_git_repo():
+            use_git = prompt_user_yesno(
                 non_interactive,
                 "This is a git repository. Do you want cfbs to make commits to it?",
             )
         else:
-            do_git = prompt_user_yesno(
+            use_git = prompt_user_yesno(
                 non_interactive,
                 "Do you want cfbs to initialize a git repository and make commits to it?",
             )
-    else:
-        assert do_git in ("yes", "no")
-        do_git = True if do_git == "yes" else False
-
-    if do_git is True:
-        if not git_exists():
-            print("Command 'git' was not found")
-            return 1
 
+    if use_git is True:
         user_name = get_args().git_user_name
-        if not user_name:
-            user_name = git_get_config("user.name")
-            user_name = prompt_user(
-                non_interactive,
-                "Please enter user name to use for git commits",
-                default=user_name or "cfbs",
-            )
-
         user_email = get_args().git_user_email
-        if not user_email:
-            user_email = git_get_config("user.email")
-            node_name = os.uname().nodename
-            user_email = prompt_user(
-                non_interactive,
-                "Please enter user email to use for git commits",
-                default=user_email or ("cfbs@%s" % node_name),
-            )
-
-        if not is_git:
-            try:
-                git_init(user_name, user_email, description)
-            except CFBSGitError as e:
-                print(str(e))
-                return 1
-        else:
-            if not git_set_config("user.name", user_name) or not git_set_config(
-                "user.email", user_email
-            ):
-                print("Failed to set Git user name and email")
-                return 1
+        git_configure_and_initialize(
+            user_name, user_email, non_interactive, description
+        )
 
-    config["git"] = do_git
+    config["git"] = use_git
 
     data = pretty(config, CFBS_DEFAULT_SORTING_RULES) + "\n"
     with open(cfbs_filename(), "w") as f:
         f.write(data)
     assert is_cfbs_repo()
 
-    if do_git:
+    if use_git:
         try:
             git_commit_maybe_prompt(
                 "Initialized a new CFEngine Build project",
                 non_interactive,
                 [cfbs_filename()],
             )
-        except CFBSGitError as e:
-            print(str(e))
+        except CFBSGitError:
             os.unlink(cfbs_filename())
-            return 1
+            raise
 
     print(
-        "Initialized an empty project called '{}' in '{}'".format(name, cfbs_filename())
+        "Initialized an empty project called '{}' in '{}'".format(
+            project_name, cfbs_filename()
+        )
     )
 
     """

cfbs/git.py

@@ -14,7 +14,8 @@
 from subprocess import check_call, check_output, run, PIPE, DEVNULL, CalledProcessError
 from typing import Iterable, Union
 
-from cfbs.utils import are_paths_equal
+from cfbs.prompts import prompt_user
+from cfbs.utils import CFBSExitError, are_paths_equal
 
 
 class CFBSGitError(Exception):
@@ -115,6 +116,38 @@ def git_init(user_name=None, user_email=None, description=None, initial_branch="
             f.write(description + "\n")
 
 
+def git_configure_and_initialize(
+    user_name=None, user_email=None, non_interactive=False, description=None
+):
+    if not git_exists():
+        raise CFBSExitError("Command 'git' was not found")
+
+    if not user_name:
+        user_name = git_get_config("user.name")
+        user_name = prompt_user(
+            non_interactive,
+            "Please enter user name to use for git commits",
+            default=user_name or "cfbs",
+        )
+
+    if not user_email:
+        user_email = git_get_config("user.email")
+        node_name = os.uname().nodename
+        user_email = prompt_user(
+            non_interactive,
+            "Please enter user email to use for git commits",
+            default=user_email or ("cfbs@%s" % node_name),
+        )
+
+    if not is_git_repo():
+        git_init(user_name, user_email, description)
+    else:
+        if not git_set_config("user.name", user_name) or not git_set_config(
+            "user.email", user_email
+        ):
+            raise CFBSExitError("Failed to set Git user name and email")
+
+
 def git_commit(
     commit_msg,
     edit_commit_msg=False,

cfbs/git_magic.py

@@ -98,13 +98,13 @@ def decorated_fn(*args, **kwargs):
             if not should_commit:
                 return ret
 
-            if do_git == "yes":
+            if do_git is True:
                 if not is_git_repo():
                     log.error(
                         "Used '--git=yes' option on what appears to not be a git repository"
                     )
                     return ret
-            elif do_git == "no":
+            elif do_git is False:
                 return ret
             else:
                 assert do_git is None

cfbs/main.py

@@ -10,6 +10,7 @@
 import pathlib
 from typing import Union
 
+from cfbs.git import CFBSGitError
 from cfbs.result import Result
 from cfbs.validate import validate_index_string
 from cfbs.version import string as version
@@ -163,6 +164,7 @@ def _main() -> Union[int, Result]:
             index=args.index,
             masterfiles=args.masterfiles,
             non_interactive=args.non_interactive,
+            use_git=args.git,
         )
 
     if args.command == "search":
@@ -284,6 +286,9 @@ def main() -> int:
     except CFBSNetworkError as e:
         print("Error: " + str(e))
         return 1
+    except CFBSGitError as e:
+        print("Error: " + str(e))
+        return 1
     # AssertionError and CFBSProgrammerError are not expected, print extra info:
     except AssertionError as e:
         tb = traceback.extract_tb(e.__traceback__)