refactor argument validation and help to utilize argparse

cli/lesspass/__init__.py

@@ -1 +1,2 @@
-name = "lesspass"
+name = "lesspass"
+description = "Lesspass is a stateless password manager."

cli/lesspass/cli.py

@@ -1,30 +1,104 @@
 import argparse
 import os
 
+from lesspass import version
+from lesspass import name
+from lesspass import description
+
+examples = \
+"""
+examples:
+  # no symbols
+  lesspass site login masterpassword --no-symbols
+
+  # no symbols shortcut
+  lesspass site login masterpassword -lud
+
+  # only digits and length of 8
+  lesspass site login masterpassword -d -L8
+
+  # master password in env variable
+  LESSPASS_MASTER_PASSWORD="masterpassword" lesspass site login
+"""
+
+copyright = \
+"""
+copyright:
+  Copyright © 2018 Guillaume Vincent <contact@lesspass.com>.  License GPLv3: GNU GPL version 3 <https://gnu.org/licenses/gpl.html>.
+  This is free software: you are free to change and redistribute it.  There is NO WARRANTY, to the extent permitted by law
+"""
 
 def parse_args(args):
-    parser = argparse.ArgumentParser(add_help=False)
-    parser.add_argument("site", nargs="?")
-    parser.add_argument("login", nargs="?")
+    parser = argparse.ArgumentParser(
+        # we override usage here to match original help output
+        # and to indicate SITE as a required argument, either via
+        # cli or via the prompt flag
+        usage="lesspass SITE [LOGIN] [MASTER_PASSWORD] [OPTIONS]",
+        description=description,
+        epilog=examples+copyright,
+        formatter_class=argparse.RawDescriptionHelpFormatter)
+    parser.add_argument("-v", "--version", action="version",
+                        version=version.__version__)
+    # technically this is required, but we can't require it here because
+    # the user can still provide this via --prompt
+    parser.add_argument("site",
+                        nargs="?",
+                        help="site used in the password generation. " +
+                        "(required)")
+    parser.add_argument("login", nargs="?",
+                        help="login used in the password generation." +
+                        " Default to '' if not provided")
     parser.add_argument(
         "master_password",
         default=os.environ.get("LESSPASS_MASTER_PASSWORD", None),
         nargs="?",
+        help="master password used in password generation. Default " +
+        "to LESSPASS_MASTER_PASSWORD env variable or prompt."
     )
-    parser.add_argument("-l", "--lowercase", dest="l", action="store_true")
-    parser.add_argument("-u", "--uppercase", dest="u", action="store_true")
-    parser.add_argument("-d", "--digits", dest="d", action="store_true")
-    parser.add_argument("-s", "--symbols", dest="s", action="store_true")
-    parser.add_argument("--no-lowercase", dest="nl", action="store_true")
-    parser.add_argument("--no-uppercase", dest="nu", action="store_true")
-    parser.add_argument("--no-digits", dest="nd", action="store_true")
-    parser.add_argument("--no-symbols", dest="ns", action="store_true")
-    parser.add_argument("-L", "--length", default=16, type=int)
-    parser.add_argument("-C", "--counter", default=1, type=int)
-    parser.add_argument("-p", "--prompt", dest="prompt", action="store_true")
+    parser.add_argument("-L", "--length", default=16, type=int,
+                        help="password length (default: 16, max: 35)")
+    parser.add_argument("-C", "--counter", default=1, type=int,
+                        help="password counter (default: 1)")
+    parser.add_argument("-p", "--prompt", dest="prompt", 
+                        action="store_true",
+                        help="prompt for values interactively")
     parser.add_argument(
-        "-c", "--copy", "--clipboard", dest="clipboard", action="store_true"
+        "-c", "--copy", "--clipboard", dest="clipboard", action="store_true",
+        help="attempt to copy to password to clipboard"
     )
-    parser.add_argument("-h", "--help", action="store_true")
-    parser.add_argument("-v", "--version", action="store_true")
+
+    lowercase_excl = parser.add_mutually_exclusive_group()
+    lowercase_excl.add_argument("-l", "--lowercase",
+                                help="add lowercase in password",
+                                dest="l", 
+                                action="store_true")
+    lowercase_excl.add_argument("--no-lowercase",
+                                help="remove lowercase from password",
+                                dest="nl", 
+                                action="store_true")
+
+    uppercase_excl = parser.add_mutually_exclusive_group()
+    uppercase_excl.add_argument("-u", "--uppercase", dest="u",
+                                help="add uppercase in password",
+                                action="store_true")
+    uppercase_excl.add_argument("--no-uppercase", dest="nu",
+                                help="remove uppercase from password",
+                                action="store_true")
+
+    digits_excl = parser.add_mutually_exclusive_group()
+
+    digits_excl.add_argument("-d", "--digits", dest="d",
+                             help="add digits in password",
+                             action="store_true")
+    digits_excl.add_argument("--no-digits", dest="nd",
+                             help="remove digits from password",                            
+                             action="store_true")
+
+    symbols_excl = parser.add_mutually_exclusive_group()
+    symbols_excl.add_argument("-s", "--symbols", dest="s",
+                              help="add symbols in password",
+                              action="store_true")
+    symbols_excl.add_argument("--no-symbols", dest="ns",
+                              help="remove symbols from password",
+                              action="store_true")
     return parser.parse_args(args)

cli/lesspass/core.py

@@ -6,35 +6,36 @@ import signal
 
 from lesspass.version import __version__
 from lesspass.cli import parse_args
-from lesspass.help import print_help
-from lesspass.validator import validate_args
 from lesspass.profile import create_profile
 from lesspass.password import generate_password
-from lesspass.clipboard import copy
+from lesspass.clipboard import copy, get_system_copy_command
 
 signal.signal(signal.SIGINT, lambda s, f: sys.exit(0))
 
 def main(args=sys.argv[1:]):
     args = parse_args(args)
-    if args.version:
-        print(__version__)
-        sys.exit(0)
-    error, help_message = validate_args(args)
-    if args.help:
-        print_help(help_message, long=True)
-        sys.exit(0)
-    if error:
-        print_help(help_message)
-        sys.exit(0)
-    profile, master_password = create_profile(args)
+    if args.clipboard and not get_system_copy_command():
+        print("ERROR To use the option -c (--copy) you need pbcopy " +
+              "on OSX, xsel or xclip on Linux, and clip on Windows")
+        sys.exit(3)
 
     if args.prompt:
-        profile["site"] = getpass.getpass("Site: ")
-        profile["login"] = getpass.getpass("Login: ")
-
-    if not master_password:
-        master_password = getpass.getpass("Master Password: ")
+        args.site = getpass.getpass("Site: ")
+        args.login = getpass.getpass("Login: ")
+    if not args.master_password:
+        args.master_password = getpass.getpass("Master Password: ")
+    
+    # if by this point we don't have SITE or the master password,
+    # we should stop.
+    if not args.site:
+        print("ERROR argument SITE is required but was not provided.")
+        sys.exit(4)
+    if not args.master_password:
+        print("ERROR argument MASTER_PASSWORD is required but " + 
+              "was not provided")
+        sys.exit(5)
 
+    profile, master_password = create_profile(args)
     generated_password = generate_password(profile, master_password)
 
     if args.clipboard:

cli/lesspass/help.py

@@ -1,74 +0,0 @@
-usage = "Usage: lesspass SITE [LOGIN] [MASTER_PASSWORD] [OPTIONS]"
-
-
-def get_short_help(help_message):
-    return "%s\nErrors:\n%s\nTry 'lesspass --help' for more information." % (
-        usage,
-        help_message,
-    )
-
-
-def get_long_help():
-    return """Name:
-
-  LessPass - stateless password generator
-
-Usage:
-
-  lesspass SITE [LOGIN] [MASTER_PASSWORD] [OPTIONS]
-
-Arguments:
-
-  SITE                site used in the password generation (required)
-  LOGIN               login used in the password generation
-                      default to '' if not provided
-  MASTER_PASSWORD     master password used in password generation
-                      default to LESSPASS_MASTER_PASSWORD env variable or prompt
-
-Options:
-
-  -l, --lowercase      add lowercase in password
-  -u, --uppercase      add uppercase in password
-  -d, --digits         add digits in password
-  -s, --symbols        add symbols in password
-  -L, --length         int (default 16, max 35)
-  -C, --counter        int (default 1)
-  -p, --prompt         interactively prompt SITE and LOGIN (prevent leak to shell history)
-  --no-lowercase       remove lowercase from password
-  --no-uppercase       remove uppercase from password
-  --no-digits          remove digits from password
-  --no-symbols         remove symbols from password
-  -c, --clipboard      copy generated password to clipboard rather than displaying it.
-                       Need pbcopy (OSX), xsel or xclip (Linux) or clip (Windows).
-  -v, --version        lesspass version number
-
-Examples:
-
-  # no symbols
-  lesspass site login masterpassword --no-symbols
-
-  # no symbols shortcut
-  lesspass site login masterpassword -lud
-
-  # only digits and length of 8
-  lesspass site login masterpassword -d -L8
-
-  # master password in env variable
-  LESSPASS_MASTER_PASSWORD="masterpassword" lesspass site login
-
-Copyright:
-
-  Copyright © 2018 Guillaume Vincent <contact@lesspass.com>.  License GPLv3: GNU GPL version 3 <https://gnu.org/licenses/gpl.html>.
-  This is free software: you are free to change and redistribute it.  There is NO WARRANTY, to the extent permitted by law."""
-
-
-def print_help(help_message, long=False):
-    usage = "Usage: lesspass SITE [LOGIN] [MASTER_PASSWORD] [OPTIONS]"
-    short_help = "%s\nErrors:\n%s\nTry 'lesspass --help' for more information." % (
-        usage,
-        help_message,
-    )
-    if long:
-        print(get_long_help())
-    else:
-        print(short_help)

cli/lesspass/validator.py

@@ -1,68 +0,0 @@
-from lesspass.clipboard import get_system_copy_command
-
-
-class NoOppositeRules(object):
-    def __init__(self, args):
-        self.args = args
-        self.error_message = ""
-
-    def is_valid(self):
-        is_valid = True
-        if self.args.l and self.args.nl:
-            self.error_message += (
-                " * Can't have -l (--lowercase) and --no-lowercase at the same time"
-            )
-            is_valid = False
-        if self.args.u and self.args.nu:
-            self.error_message += (
-                " * Can't have -u (--uppercase) and --no-uppercase at the same time"
-            )
-            is_valid = False
-        if self.args.d and self.args.nd:
-            self.error_message += (
-                " * Can't have -d (--digits) and --no-digits at the same time"
-            )
-            is_valid = False
-        if self.args.s and self.args.ns:
-            self.error_message += (
-                " * Can't have -s (--symbols) and --no-symbols at the same time"
-            )
-            is_valid = False
-        return is_valid
-
-
-class DefaultParameters(object):
-    def __init__(self, args):
-        self.args = args
-        self.error_message = ""
-
-    def is_valid(self):
-        is_valid = True
-        if not self.args.site and not self.args.prompt:
-            self.error_message += " * SITE is a required argument (unless in interactive mode with --prompt)"
-            is_valid = False
-        return is_valid
-
-
-class ClipboardAvailable(object):
-    def __init__(self, args):
-        self.args = args
-        self.error_message = ""
-
-    def is_valid(self):
-        is_valid = True
-        if self.args.clipboard and not get_system_copy_command():
-            self.error_message += " * To use the option -c (--copy) you need pbcopy on OSX, xsel or xclip on Linux and clip on Windows"
-            is_valid = False
-        return is_valid
-
-
-def validate_args(args):
-    rules = [NoOppositeRules(args), DefaultParameters(args), ClipboardAvailable(args)]
-    error = False
-    error_message = ""
-    for rule in rules:
-        if not rule.is_valid():
-            error = True
-            error_message += "%s\n" % rule.error_message
-    return error, error_message

cli/setup.py

@@ -1,7 +1,7 @@
 import setuptools
 
 from lesspass.version import __version__
-from lesspass.help import get_long_help
+from lesspass import description
 
 
 setuptools.setup(
@@ -11,7 +11,7 @@ setuptools.setup(
     author='Guillaume Vincent',
     author_email='contact@lesspass.com',
     description='LessPass stateless password generator',
-    long_description=get_long_help(),
+    long_description=description,
     install_requires=[],
     entry_points="""
         [console_scripts]

cli/tests/test_cli.py

@@ -6,14 +6,6 @@ from lesspass.cli import parse_args
 
 
 class TestParseArgs(unittest.TestCase):
-    def test_parse_args_version(self):
-        self.assertTrue(parse_args(["--version"]).version)
-        self.assertTrue(parse_args(["-v"]).version)
-
-    def test_parse_args_help(self):
-        self.assertTrue(parse_args(["--help"]).help)
-        self.assertTrue(parse_args(["-h"]).help)
-
     def test_parse_args_site(self):
         self.assertEqual(parse_args(["site"]).site, "site")
 

cli/tests/test_validator.py

@@ -1,59 +1,59 @@
-import unittest
-
-from lesspass.cli import parse_args
-from lesspass.validator import validate_args
-
-
-class TestValidateArgs(unittest.TestCase):
-    def test_validate_args_no_opposite_rules_lowercase(self):
-        error, message = validate_args(parse_args(["site", "-l", "--no-lowercase"]))
-        self.assertTrue(error)
-        self.assertTrue(
-            "Can't have -l (--lowercase) and --no-lowercase at the same time" in message
-        )
-
-    def test_validate_args_no_opposite_rules_uppercase(self):
-        error, message = validate_args(parse_args(["site", "-u", "--no-uppercase"]))
-        self.assertTrue(error)
-        self.assertTrue(
-            "Can't have -u (--uppercase) and --no-uppercase at the same time" in message
-        )
-
-    def test_validate_args_no_opposite_rules_digits(self):
-        error, message = validate_args(parse_args(["site", "-d", "--no-digits"]))
-        self.assertTrue(error)
-        self.assertTrue(
-            "Can't have -d (--digits) and --no-digits at the same time" in message
-        )
-
-    def test_validate_args_no_opposite_rules_symbols(self):
-        error, message = validate_args(parse_args(["site", "-s", "--no-symbols"]))
-        self.assertTrue(error)
-        self.assertTrue(
-            "Can't have -s (--symbols) and --no-symbols at the same time" in message
-        )
-
-    def test_validate_args_concat_errors(self):
-        _, message = validate_args(
-            parse_args(["site", "-u", "--no-uppercase", "-l", "--no-lowercase"])
-        )
-        self.assertTrue(
-            "Can't have -l (--lowercase) and --no-lowercase at the same time" in message
-        )
-        self.assertTrue(
-            "Can't have -u (--uppercase) and --no-uppercase at the same time" in message
-        )
-
-    def test_validate_args_no_site(self):
-        error, message = validate_args(parse_args([]))
-        self.assertTrue(error)
-        self.assertTrue(
-            "SITE is a required argument" in message
-        )
-
-    def test_validate_args_site_optional_with_prompt(self):
-        error, message = validate_args(parse_args(["--prompt"]))
-        self.assertFalse(error)
-        self.assertTrue(
-            "SITE is a required argument" not in message
-        )
+# import unittest
+
+# from lesspass.cli import parse_args
+# from lesspass.validator import validate_args
+
+
+# class TestValidateArgs(unittest.TestCase):
+#     def test_validate_args_no_opposite_rules_lowercase(self):
+#         error, message = validate_args(parse_args(["site", "-l", "--no-lowercase"]))
+#         self.assertTrue(error)
+#         self.assertTrue(
+#             "Can't have -l (--lowercase) and --no-lowercase at the same time" in message
+#         )
+
+#     def test_validate_args_no_opposite_rules_uppercase(self):
+#         error, message = validate_args(parse_args(["site", "-u", "--no-uppercase"]))
+#         self.assertTrue(error)
+#         self.assertTrue(
+#             "Can't have -u (--uppercase) and --no-uppercase at the same time" in message
+#         )
+
+#     def test_validate_args_no_opposite_rules_digits(self):
+#         error, message = validate_args(parse_args(["site", "-d", "--no-digits"]))
+#         self.assertTrue(error)
+#         self.assertTrue(
+#             "Can't have -d (--digits) and --no-digits at the same time" in message
+#         )
+
+#     def test_validate_args_no_opposite_rules_symbols(self):
+#         error, message = validate_args(parse_args(["site", "-s", "--no-symbols"]))
+#         self.assertTrue(error)
+#         self.assertTrue(
+#             "Can't have -s (--symbols) and --no-symbols at the same time" in message
+#         )
+
+#     def test_validate_args_concat_errors(self):
+#         _, message = validate_args(
+#             parse_args(["site", "-u", "--no-uppercase", "-l", "--no-lowercase"])
+#         )
+#         self.assertTrue(
+#             "Can't have -l (--lowercase) and --no-lowercase at the same time" in message
+#         )
+#         self.assertTrue(
+#             "Can't have -u (--uppercase) and --no-uppercase at the same time" in message
+#         )
+
+#     def test_validate_args_no_site(self):
+#         error, message = validate_args(parse_args([]))
+#         self.assertTrue(error)
+#         self.assertTrue(
+#             "SITE is a required argument" in message
+#         )
+
+#     def test_validate_args_site_optional_with_prompt(self):
+#         error, message = validate_args(parse_args(["--prompt"]))
+#         self.assertFalse(error)
+#         self.assertTrue(
+#             "SITE is a required argument" not in message
+#         )