-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
Clearer errorr message for "Cannot parse" when targetting multiple version #3294
Comments
Thanks for submitting the issue! I'm all for a better error message 👍 |
hey @felix-hilden, i would like to work on this, and add the grammar version to the error message. grammars = get_grammars(set(target_versions))
errors = {}
for grammar in grammars:
drv = driver.Driver(grammar)
try:
result = drv.parse_string(src_txt, True)
break
except ParseError as pe:
lineno, column = pe.context[1]
lines = src_txt.splitlines()
try:
faulty_line = lines[lineno - 1]
except IndexError:
faulty_line = "<line number missing in source>"
errors[grammar.version] = InvalidInput(
f"Cannot parse: {lineno}:{column}: {faulty_line}" # 103
) could you assign me? |
I think this issue should be marked closed, correct me if I am wrong |
hey, usually an issue is closed when a PR is merged. this is done automatically if the PR contains something like |
ok, cool. Looks like it's left for a pending reviewer to take a look |
Hi @plannigan, I would like to work on this issue could you please assign me this. |
We don't usually assign issues. If you have a suggested solution, please open a PR and we'll take a look. |
I also don't have permissions to assign people to issues 😄. |
hey guys, i offered a solution to this issue last year: alas - it was disapproved, despite the fact that it fixed this issue. |
I might try a new approach to this. |
I've had another go at this, taking into account the feedback for the previous PR. I was looking into the second part of the request about noting that syntax could be valid across other target versions, and I'm unable to reproduce this issue. As far as I can see, this error only occurs if ALL versions are unable to parse the line, so we shouldn't need this error to handle a case where some versions parse and others don't. |
Given how old the ticket is, I don't remember exactly what the originally configured python versions were. But I feel like the content under "Describe alternatives you've considered" was more of "maybe X is happening" rather than "I know X is happening". So if there are different/better error messages when one configured parser can parse, but a different configured parser can't, then that is fine with me. |
Is your feature request related to a problem? Please describe.
I was working on a project that is only intended to run on Python 3.10. However, the
black
configuration incorrectly listed multiple target versions because I copied the file from a different project.When I tried to format a file with a match statement, the error was
I was initially confused as I knew that
black
had added Python 3.10 support. Then I noticed my mistake. By saying that the code should target a multiple Python versions, using a match statement would not be valid.Describe the solution you'd like
If
black
encounters a parsing error and it was configured to target multiple Python versions, I think it would be helpful to mention that the syntax "might be valid, but not across all target versions". This would be a hint to the user to think about if they should be using that syntax or thatblack
is misconfigured (as it was in my case).Describe alternatives you've considered
It would require more work, but the parser could back track and attempt each of the parser for each of the listed target versions. That would allow
black
provide an even more specific error message. Something like "this is valid for Python 3.10, but targeting Python 3.10 and 3.9", while also being able to identify an invalid syntax.The text was updated successfully, but these errors were encountered: