From 178d3ecb00963148548269b34433087f3c5989e3 Mon Sep 17 00:00:00 2001 From: Baizley Date: Fri, 20 Dec 2019 21:16:41 +0100 Subject: [PATCH 1/4] Add unit test for handling getopt error. --- gramps/cli/test/argparser_test.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/gramps/cli/test/argparser_test.py b/gramps/cli/test/argparser_test.py index 5a9e29cc0..0b2c607d7 100644 --- a/gramps/cli/test/argparser_test.py +++ b/gramps/cli/test/argparser_test.py @@ -68,5 +68,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() From 1b53a3948aa7d3be92bbabda0c2265a3ea8f2564 Mon Sep 17 00:00:00 2001 From: Baizley Date: Fri, 20 Dec 2019 21:27:37 +0100 Subject: [PATCH 2/4] Simplify handling of getops error. --- gramps/cli/argparser.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/gramps/cli/argparser.py b/gramps/cli/argparser.py index 655bf2cb7..f82445112 100644 --- a/gramps/cli/argparser.py +++ b/gramps/cli/argparser.py @@ -242,24 +242,25 @@ class ArgParser: try: options, leftargs = getopt.getopt(self.args[1:], SHORTOPTS, LONGOPTS) - except getopt.GetoptError as msg: + except getopt.GetoptError as error: # 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 - 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." - ) % cliargs)] + translated_error_message = _( + "Error parsing the arguments: %s \n" + "Type gramps --help for an overview of " + "commands, or read the manual pages." + ) % cli_args + + self.errors += [( + _('Error parsing the arguments'), + str(error) + '\n' + translated_error_message + )] + return # Some args can work on a list of databases: From 8a5c8ffbc96f87613c0dc552bf00d5f0f710b3ab Mon Sep 17 00:00:00 2001 From: Baizley Date: Fri, 20 Dec 2019 21:47:30 +0100 Subject: [PATCH 3/4] Expand unit test for Gramps parsing error. --- gramps/cli/test/argparser_test.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/gramps/cli/test/argparser_test.py b/gramps/cli/test/argparser_test.py index 0b2c607d7..fccb82619 100644 --- a/gramps/cli/test/argparser_test.py +++ b/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') - assert not bad, ap.errors - assert ap.auto_accept + bad, ap = self.triggers_option_error('-y') + + 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') From 8ca2ff5dad46ae57964134a3c47911214567b0a5 Mon Sep 17 00:00:00 2001 From: Baizley Date: Fri, 20 Dec 2019 22:05:40 +0100 Subject: [PATCH 4/4] Refactor error construction into a common method. --- gramps/cli/argparser.py | 63 +++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/gramps/cli/argparser.py b/gramps/cli/argparser.py index f82445112..8fdb70546 100644 --- a/gramps/cli/argparser.py +++ b/gramps/cli/argparser.py @@ -242,24 +242,14 @@ class ArgParser: try: options, leftargs = getopt.getopt(self.args[1:], SHORTOPTS, LONGOPTS) - except getopt.GetoptError as error: - # 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 - translated_error_message = _( - "Error parsing the arguments: %s \n" - "Type gramps --help for an overview of " - "commands, or read the manual pages." - ) % cli_args - - self.errors += [( - _('Error parsing the arguments'), - str(error) + '\n' + translated_error_message - )] + except getopt.GetoptError as getopt_error: + self.errors.append( + self.construct_error( + "Type gramps --help for an overview of " + "commands, or read the manual pages.", + error=getopt_error + ) + ) return @@ -460,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. - # 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 - 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)] + self.errors.append( + self.construct_error( + "To use in the command-line mode, supply at " + "least one input file to process." + ) + ) + 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 #-------------------------------------------------------------------------