Merge pull request #9457 from hssyoo/reduce-invalid-choice-output

[v2] Reduce invalid choice output

Author: kdaily

awscli/argparser.py

@@ -65,10 +65,6 @@ def choices(self, val):
 class CLIArgParser(argparse.ArgumentParser):
     Formatter = argparse.RawTextHelpFormatter
 
-    # When displaying invalid choice error messages,
-    # this controls how many options to show per line.
-    ChoicesPerLine = 2
-
     def _check_value(self, action, value):
         """
         It's probably not a great idea to override a "hidden" method
@@ -77,15 +73,10 @@ def _check_value(self, action, value):
         """
         # converted value must be one of the choices (if specified)
         if action.choices is not None and value not in action.choices:
-            msg = ['Invalid choice, valid choices are:\n']
-            for i in range(len(action.choices))[:: self.ChoicesPerLine]:
-                current = []
-                for choice in action.choices[i : i + self.ChoicesPerLine]:
-                    current.append('%-40s' % choice)
-                msg.append(' | '.join(current))
+            msg = [f"Found invalid choice '{value}'\n"]
             possible = get_close_matches(value, action.choices, cutoff=0.8)
             if possible:
-                extra = ['\n\nInvalid choice: %r, maybe you meant:\n' % value]
+                extra = ['Maybe you meant:\n']
                 for word in possible:
                     extra.append('  * %s' % word)
                 msg.extend(extra)
@@ -126,8 +117,8 @@ def error(self, message):
         should raise an exception.
         """
         usage_message = self.format_usage()
-        error_message = f'{self.prog}: error: {message}\n'
-        raise ArgParseException(f'{usage_message}\n{error_message}')
+        error_message = f'{self.prog}: error: {message}'
+        raise ArgParseException(f'{error_message}\n\n{usage_message}')
 
 
 class MainArgParser(CLIArgParser):

tests/functional/docs/test_help_output.py

@@ -217,7 +217,7 @@ def assert_command_does_not_exist(self, service, command):
         self.assertEqual(cr, 252)
         # We should see an error message complaining about
         # an invalid choice because the operation has been removed.
-        self.assertIn('argument operation: Invalid choice', stderr.getvalue())
+        self.assertIn('argument operation: Found invalid choice', stderr.getvalue())
 
     def test_ses_deprecated_commands(self):
         self.driver.main(['ses', 'help'])

tests/functional/ec2instanceconnect/test_ssh.py

@@ -886,7 +886,7 @@ def test_command_fails_when_eice_connection_type_and_no_private_ip(
                     "test",
                 ],
                 252,
-                "argument --connection-type: Invalid choice, valid choices are:",
+                "argument --connection-type: Found invalid choice 'test'",
                 id='Failure: Customer must provide connection-type when IP defined',
             ),
             pytest.param(

tests/functional/kinesis/test_remove_operations.py

@@ -15,5 +15,5 @@
 
 def test_subscribe_to_shard_removed():
     result = CLIRunner().run(['kinesis', 'subscribe-to-shard', 'help'])
-    expected_error = 'argument operation: Invalid choice, valid choices are:'
+    expected_error = "argument operation: Found invalid choice 'subscribe-to-shard'"
     assert expected_error in result.stderr

tests/functional/lex/test_remove_operations.py

@@ -15,5 +15,5 @@
 
 def test_start_conversation_removed():
     result = CLIRunner().run(['lexv2-runtime', 'start-conversation', 'help'])
-    expected_error = 'argument operation: Invalid choice, valid choices are:'
+    expected_error = "argument operation: Found invalid choice 'start-conversation'"
     assert expected_error in result.stderr

tests/functional/servicecatalog/test_generate_createproduct.py

@@ -198,7 +198,7 @@ def test_invalid_product_type(self):
         self.assert_params_for_cmd(
             self.cmd_line,
             expected_rc=252,
-            stderr_contains='--product-type: Invalid choice',
+            stderr_contains='--product-type: Found invalid choice',
         )
 
     def test_generate_product_invalid_provisioning_artifact_type(self):
@@ -208,5 +208,5 @@ def test_generate_product_invalid_provisioning_artifact_type(self):
         self.assert_params_for_cmd(
             self.cmd_line,
             expected_rc=252,
-            stderr_contains='--provisioning-artifact-type: Invalid choice',
+            stderr_contains='--provisioning-artifact-type: Found invalid choice',
         )

tests/functional/servicecatalog/test_generate_createprovisioningartifact.py

@@ -105,7 +105,7 @@ def test_generate_provisioning_artifact_invalid_pa_type(self):
         self.assert_params_for_cmd(
             self.cmd_line,
             expected_rc=252,
-            stderr_contains='--provisioning-artifact-type: Invalid choice',
+            stderr_contains='--provisioning-artifact-type: Found invalid choice',
         )
 
     def test_generate_provisioning_artifact_missing_file_path(self):

tests/unit/test_clidriver.py

@@ -493,10 +493,10 @@ def test_unknown_command_suggests_help(self):
         )
         self.assertEqual(rc, 252)
         # Tell the user what went wrong.
-        self.assertIn("Invalid choice: 'list-objecst'", self.stderr.getvalue())
+        self.assertIn("Found invalid choice 'list-objecst'", self.stderr.getvalue())
         # Offer the user a suggestion.
         self.assertIn(
-            "maybe you meant:\n\n  * list-objects", self.stderr.getvalue()
+            "Maybe you meant:\n\n  * list-objects", self.stderr.getvalue()
         )