From cf252b57101670c769c02c8818e89321daea20c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89douard=20Lopez?=
 <edouard-lopez@users.noreply.github.com>
Date: Fri, 12 Apr 2019 21:16:48 +0200
Subject: [PATCH] Improve min length code (#411)

* use `choices` attribute from parser.add_argument() instead of complex actions override

* use metavar to improve --help output

* use custom 'range_type' for length argument
---
 cli/lesspass/actions.py      | 19 -------------------
 cli/lesspass/cli.py          | 16 ++++++++++------
 cli/tests/test_functional.py | 14 +++++++++++---
 3 files changed, 21 insertions(+), 28 deletions(-)
 delete mode 100644 cli/lesspass/actions.py

diff --git a/cli/lesspass/actions.py b/cli/lesspass/actions.py
deleted file mode 100644
index 684455f..0000000
--- a/cli/lesspass/actions.py
+++ /dev/null
@@ -1,19 +0,0 @@
-import argparse
-
-
-class Range(argparse.Action):
-    def __init__(self, min=None, max=None, *args, **kwargs):
-        self.min = min
-        self.max = max
-        kwargs["metavar"] = "[%d-%d]" % (self.min, self.max)
-        super(Range, self).__init__(*args, **kwargs)
-
-    def __call__(self, parser, namespace, value, option_string=None):
-        if not (self.min <= value <= self.max):
-            msg = "invalid choice: %r (choose from %d to %d)" % (
-                value,
-                self.min,
-                self.max,
-            )
-            raise argparse.ArgumentError(self, msg)
-        setattr(namespace, self.dest, value)
diff --git a/cli/lesspass/cli.py b/cli/lesspass/cli.py
index 8734894..569c873 100644
--- a/cli/lesspass/cli.py
+++ b/cli/lesspass/cli.py
@@ -1,7 +1,6 @@
 import argparse
 import os
 
-from lesspass import actions
 from lesspass import version
 from lesspass import name
 from lesspass import description
@@ -27,6 +26,12 @@ copyright:
   This is free software: you are free to change and redistribute it.  There is NO WARRANTY, to the extent permitted by law
 """
 
+def range_type(value_string):
+    value = int(value_string)
+    if value not in range(5, 35+1):
+        raise argparse.ArgumentTypeError("%s is out of range, choose in [5-35]" % value)
+    return value
+
 
 def parse_args(args):
     parser = argparse.ArgumentParser(
@@ -54,11 +59,10 @@ def parse_args(args):
         "-L",
         "--length",
         default=16,
-        action=actions.Range,
-        min=5,
-        max=35,
-        type=int,
-        help="password length (default: 16, min:5, max: 35)",
+        choices=range(5, 35+1),
+        type=range_type,
+        help="password length (default: 16, min: 5, max: 35)",
+        metavar='[5-35]'
     )
     parser.add_argument(
         "-C", "--counter", default=1, type=int, help="password counter (default: 1)"
diff --git a/cli/tests/test_functional.py b/cli/tests/test_functional.py
index 0dde8b4..0bd6498 100644
--- a/cli/tests/test_functional.py
+++ b/cli/tests/test_functional.py
@@ -5,12 +5,20 @@ import time
 
 import sys
 
+import argparse
+from lesspass.cli import range_type
 
 class TestFunctional(unittest.TestCase):
     def test_length_below_the_minimum(self):
         p = pexpect.spawn(
             "python3 lesspass/core.py  site login masterpassword --lowercase --digits --length 2"
         )
-        self.assertTrue(
-            "error: argument -L/--length: invalid choice: 2" in str(p.read())
-        )
+        output = p.read().decode()
+
+        self.assertTrue("error: argument -L/--length: 2 is out of range, choose in [5-35]" in output)
+
+    def test_length_range_type(self):
+        self.assertEqual(range_type('5'), 5)
+        self.assertEqual(range_type('35'), 35) 
+        with self.assertRaises(argparse.ArgumentTypeError):
+            range_type('2')
\ No newline at end of file