Merge pull request #974 from Baizley/simplify-error-construction-in-argument-parser

gramps/cli/argparser.py

@@ -242,24 +242,15 @@ class ArgParser:
         try:
             options, leftargs = getopt.getopt(self.args[1:],
                                               SHORTOPTS, LONGOPTS)
-        except getopt.GetoptError as msg:
+        except getopt.GetoptError as getopt_error:
-            # Extract the arguments in the list.
+            self.errors.append(
-            # The % operator replaces the list elements
+                self.construct_error(
-            # with repr() of the list elements
-            # which is OK for latin characters,
-            # but not for non latin characters in list elements
-            cliargs = "[ "
-            for arg in range(len(self.args) - 1):
-                cliargs += self.args[arg + 1] + " "
-            cliargs += "]"
-            # Must first do str() of the msg object.
-            msg = str(msg)
-            self.errors += [(_('Error parsing the arguments'),
-                             msg + '\n' +
-                             _("Error parsing the arguments: %s \n"
                     "Type gramps --help for an overview of "
-                               "commands, or read the manual pages."
+                    "commands, or read the manual pages.",
-                              ) % cliargs)]
+                    error=getopt_error
+                )
+            )
+
             return
 
         # Some args can work on a list of databases:
@@ -459,22 +450,33 @@ class ArgParser:
                          or self.list_more
                          or self.list_table
                          or self.help)):
-            # Extract and convert to unicode the arguments in the list.
+            self.errors.append(
-            # The % operator replaces the list elements with repr() of
+                self.construct_error(
-            # the list elements, which is OK for latin characters
-            # but not for non-latin characters in list elements
-            cliargs = "[ "
-            for arg in range(len(self.args) - 1):
-                cliargs += self.args[arg + 1] + ' '
-            cliargs += "]"
-            self.errors += [(_('Error parsing the arguments'),
-                             _("Error parsing the arguments: %s \n"
                     "To use in the command-line mode, supply at "
                     "least one input file to process."
-                              ) % cliargs)]
+                )
+            )
+
         if need_to_quit:
             sys.exit(0)
 
+    def construct_error(self, suggestion_message, error=None):
+        # Extract the arguments in the list.
+        cli_args = "[ %s ]" % " ".join(self.args[1:])
+
+        # The % operator replaces the list elements
+        # with repr() of the list elements
+        # which is OK for latin characters,
+        # but not for non latin characters in list elements
+        error_message = "Error parsing the arguments: %s \n"
+        translated_message = _(error_message + suggestion_message) % cli_args
+
+        if error:
+            translated_message = str(error) + '\n' + translated_message
+
+        return _('Error parsing the arguments'), translated_message
+
+
     #-------------------------------------------------------------------------
     # Determine the need for GUI
     #-------------------------------------------------------------------------

gramps/cli/test/argparser_test.py

@@ -41,9 +41,21 @@ class TestArgParser(unittest.TestCase):
         assert bad, ap.__dict__
 
     def test_y_shortopt_sets_auto_accept(self):
-        bad,ap = self.triggers_option_error('-y')
+        bad, ap = self.triggers_option_error('-y')
-        assert not bad, ap.errors
+
-        assert ap.auto_accept
+        self.assertFalse(bad)
+
+        expected_errors = [(
+            'Error parsing the arguments',
+            'Error parsing the arguments: [ -y ] \n' +
+            'To use in the command-line mode, supply at least one input file to process.'
+        )]
+        self.assertEqual(
+            expected_errors,
+            ap.errors
+        )
+
+        self.assertTrue(ap.auto_accept)
 
     def test_yes_longopt_sets_auto_accept(self):
         bad,ap = self.triggers_option_error('--yes')
@@ -68,5 +80,20 @@ class TestArgParser(unittest.TestCase):
         ap = self.create_parser()
         assert not ap.auto_accept
 
+    def test_exception(self):
+        argument_parser = self.create_parser("-O")
+
+        expected_errors = [(
+            'Error parsing the arguments',
+            'option -O requires argument\n'
+            'Error parsing the arguments: [ -O ] \n'
+            'Type gramps --help for an overview of commands, or read the manual pages.'
+        )]
+        self.assertEqual(
+            expected_errors,
+            argument_parser.errors
+        )
+
+
 if __name__ == "__main__":
     unittest.main()