10000 Normalise Argument Clinic error messages · Issue #107614 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Normalise Argument Clinic error messages #107614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
erlend-aasland opened this issue Aug 4, 2023 · 6 comments
Closed

Normalise Argument Clinic error messages #107614

erlend-aasland opened this issue Aug 4, 2023 · 6 comments
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor
erlend-aasland commented Aug 4, 2023 •
edited by bedevere-bot
Loading

Argument Clinic is pretty good at disallowing weird corner cases and producing error messages (there are 130 fail(..)s sprinkled around clinic.py!). Unfortunately, many of the error messages do not wrap the line or token in question, nor the function/module/class name in question in quotes. Also, some error messages contain line breaks in unexpected places.

Some examples:

Error message with unexpected line break

Clinic input:

/*[clinic input]
function
    a: int = 0
    b: int
[clinic start generated code]*/
$ python3.12 Tools/clinic/clinic.py test.c
Error in file 'test.c' on line 4:
Can't have a parameter without a default ('b')
after a parameter with a default!

Clinic input:

/*[clinic input]
function
    b: int
    *
[clinic start generated code]*/

Error message:

$ python3.12 Tools/clinic/clinic.py test.c
Error in file 'test.c' on line 5:
Function function specifies '*' without any parameters afterwards.

Suggesting to normalise Argument Clinic error messages, using the following guidelines:

  • always wrap the offending line, token, or name in quotes
  • no line break; the entire error message should be on one line

Linked PRs
@erlend-aasland erlend-aasland added type-feature A feature request or enhancement topic-argument-clinic labels Aug 4, 2023
@bedevere-bot bedevere-bot mentioned this issue Aug 4, 2023
Merged
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 4, 2023
@erlend-aasland
pythongh-107614: Normalise Argument Clinic error messages
c66a492 
- always wrap the offending line, token, or name in quotes
- always put the entire error message on one line
@erlend-aasland
Copy link
Contributor Author

In addition to normalising the error message format, I'd like to do an audit on the error message content. We've got some messages that are hard to comprehend; I think we can do better. Example:

"Function 'bar' has an unsupported group configuration. (Unexpected state 0.d)"

There are four variants on the error message above; none of them are any better.

There are also a lot of error messages where the offending line/token/name is not included, which leads to the programmer having to guess what's wrong. Example:

"Annotations must be either a name, a function call, or a string."

@erlend-aasland
Copy link
Contributor Author

There is a place where line break really could help, though:

diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py
index 979e07f13c..2fa75133da 100644
--- a/Lib/test/test_clinic.py
+++ b/Lib/test/test_clinic.py
@@ -634,13 +634,24 @@ def test_directive_preserve_output(self):
         self.assertEqual(generated, block)
 
     def test_directive_output_invalid_command(self):
-        err = (
-            "Invalid command / destination name 'cmd', must be one of:\n"
-            "preset push pop print everything cpp_if docstring_prototype "
-            "docstring_definition methoddef_define impl_prototype "
-            "parser_prototype parser_definition cpp_endif methoddef_ifndef "
-            "impl_definition"
-        )
+        err = dedent("""
+            Invalid command or destination name 'cmd'. Must be one of:
+             - preset
+             - push
+             - pop
+             - print
+             - everything
+             - cpp_if
+             - docstring_prototype
+             - docstring_definition
+             - methoddef_define
+             - impl_prototype
+             - parser_prototype
+             - parser_definition
+             - cpp_endif
+             - methoddef_ifndef
+             - impl_definition
+        """).strip()
         block = """
             /*[clinic input]
             output cmd buffer
diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py
index a50f574327..d2a96ecfa4 100755
--- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -4567,8 +4567,10 @@ def directive_output(
             return
 
         if command_or_name not in fd:
-            fail(f"Invalid command / destination name {command_or_name!r}, must be one of:\n"
-                 "preset push pop print everything", " ".join(fd))
+            allowed = ["preset", "push", "pop", "print", "everything"]
+            allowed.extend(fd)
+            fail(f"Invalid command or destination name {command_or_name!r}. "
+                 "Must be one of:\n -", "\n - ".join(allowed))
         fd[command_or_name] = d
 
     def directive_dump(self, name: str) -> None:

@AlexWaygood
Copy link
Member

There is a place where line break really could help, though:

+1, I think that would be an improvement

@erlend-aasland
Copy link
Contributor Author

There is a place where line break really could help, though:

+1, I think that would be an improvement

Added on the PR with feb6aec

erlend-aasland added a commit that referenced this issue Aug 4, 2023
@erlend-aasland @AlexWaygood
gh-107614: Normalise Argument Clinic error messages (#107615)
ac7605e 
- always wrap the offending line, token, or name in quotes
- in most cases, put the entire error message on one line

Added tests for uncovered branches that were touched by this PR.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@erlend-aasland
Copy link
Contributor Author

The error messages are now normalised with regard to formatting. Suggesting to address issues with the content of the messages in separate issues.

@erlend-aasland
Copy link
Contributor Author

Resolved with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-argument-clinic type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@erlend-aasland @AlexWaygood
0