Repository: fniessink/next-action

Issue:
Give error message when --number argument is a negative integer

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 16cf30d..b325977 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased] - 2018-07-17
+
+### Fixed
+
+- Give proper error message when the `--number` argument is smaller than one. Fixes #164.
+
 ## [1.5.3] - 2018-07-14
 
 ### Fixed
diff --git a/README.md b/README.md
index ffb50ba..b489018 100644
--- a/README.md
+++ b/README.md
@@ -448,9 +448,9 @@ To run the unit tests:
 
 ```console
 $ python -m unittest
-............................................................................................................................................................................................................................................
+.............................................................................................................................................................................................................................................
 ----------------------------------------------------------------------
-Ran 236 tests in 2.931s
+Ran 237 tests in 2.303s
 
 OK
 ```
@@ -461,9 +461,9 @@ To create the unit test coverage report run the unit tests under coverage with:
 
 ```console
 $ coverage run --branch -m unittest
-............................................................................................................................................................................................................................................
+.............................................................................................................................................................................................................................................
 ----------------------------------------------------------------------
-Ran 236 tests in 3.557s
+Ran 237 tests in 3.159s
 
 OK
 ```
@@ -475,7 +475,7 @@ $ coverage report --fail-under=100 --omit=".venv/*" --skip-covered
 Name    Stmts   Miss Branch BrPart  Cover
 -----------------------------------------
 -----------------------------------------
-TOTAL    1347      0    173      0   100%
+TOTAL    1360      0    177      0   100%
 
 25 files skipped due to complete coverage.
 ```
diff --git a/next_action/arguments/parser.py b/next_action/arguments/parser.py
index 4d9df44..5d8c8ad 100644
--- a/next_action/arguments/parser.py
+++ b/next_action/arguments/parser.py
@@ -85,7 +85,7 @@ class NextActionArgumentParser(argparse.ArgumentParser):
             "-a", "--all", default=1, action="store_const", dest="number", const=sys.maxsize,
             help="show all next actions")
         number.add_argument(
-            "-n", "--number", metavar="<number>", type=int, default=1,
+            "-n", "--number", metavar="<number>", type=number_type, default=1,
             help="number of next actions to show (default: %(default)s)")
 
     def add_filter_arguments(self) -> None:
@@ -257,6 +257,17 @@ def date_type(value: str) -> datetime.date:
     raise argparse.ArgumentTypeError("invalid date: {0}".format(value))
 
 
+def number_type(value: str) -> int:
+    """Return the value if it's positive, else raise an error."""
+    try:
+        number = int(value)
+        if number > 0:
+            return number
+    except ValueError:
+        pass
+    raise argparse.ArgumentTypeError("invalid number: {0}".format(value))
+
+
 def subset(filters: List[str], prefix: str) -> Set[str]:
     """Return a subset of the filters based on prefix."""
     return set(f.strip(prefix) for f in filters if f.startswith(prefix))


Test patch:
diff --git a/tests/unittests/arguments/test_parser.py b/tests/unittests/arguments/test_parser.py
index fed6430..e606485 100644
--- a/tests/unittests/arguments/test_parser.py
+++ b/tests/unittests/arguments/test_parser.py
@@ -212,7 +212,16 @@ class NumberTest(ParserTestCase):
         """Test that the argument parser exits if the option is faulty."""
         self.assertRaises(SystemExit, parse_arguments)
         self.assertEqual([call(USAGE_MESSAGE),
-                          call("next-action: error: argument -n/--number: invalid int value: 'not_a_number'\n")],
+                          call("next-action: error: argument -n/--number: invalid number: not_a_number\n")],
+                         mock_stderr_write.call_args_list)
+
+    @patch.object(sys, "argv", ["next-action", "--number", "-1"])
+    @patch.object(sys.stderr, "write")
+    def test_negative_number(self, mock_stderr_write):
+        """Test that the argument parser exits if the option is faulty."""
+        self.assertRaises(SystemExit, parse_arguments)
+        self.assertEqual([call(USAGE_MESSAGE),
+                          call("next-action: error: argument -n/--number: invalid number: -1\n")],
                          mock_stderr_write.call_args_list)
 
     @patch.object(sys, "argv", ["next-action", "--all"])