Skip to content

Improve error shown when there's no space between semicolon and url#592

Closed
Jackenmen wants to merge 3 commits intopypa:mainfrom
Jackenmen:issue_529
Closed

Improve error shown when there's no space between semicolon and url#592
Jackenmen wants to merge 3 commits intopypa:mainfrom
Jackenmen:issue_529

Conversation

@Jackenmen
Copy link
Copy Markdown
Contributor

@Jackenmen Jackenmen commented Sep 14, 2022

Fixes #529

I kind of wanted to also show:

Expected space and semicolon (followed by markers) or end of string

when url doesn't end with ; but this sentence seemed a bit unnatural to me and I couldn't think of a better way of phrasing it + if the user ends up not putting a space, they'll get the better message when they try for the second time anyway.

I also originally wanted to say something more similar to Python's error messages, i.e. "Maybe you meant to put a space before the semicolon?" but that doesn't quite fit with the error message format ending with "at position ..":

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/work/packaging/packaging/requirements.py", line 42, in __init__
    raise InvalidRequirement(str(e))
packaging.requirements.InvalidRequirement: Expected semicolon (followed by markers) or end of string. Maybe you meant to put a space before the semicolon?
at position 59:
    tomli @ https://github.com/hukkin/tomli/archive/master.zip; python_version < '3.11'
                                                              ^

Either way, I'm open to suggestions.

@Jackenmen Jackenmen mentioned this pull request Sep 14, 2022
Closed
@pradyunsg
Copy link
Copy Markdown
Member

pradyunsg commented Sep 24, 2022

We could add a “hint: maybe you meant…” line to the error message.

@Jackenmen
Copy link
Copy Markdown
Contributor Author

Jackenmen commented Sep 24, 2022

You mean like this?

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/work/packaging/packaging/requirements.py", line 42, in __init__
    raise InvalidRequirement(str(e))
packaging.requirements.InvalidRequirement: Expected semicolon (followed by markers) or end of string. Hint: maybe you meant to put a space before the semicolon?
at position 59:
    tomli @ https://github.com/hukkin/tomli/archive/master.zip; python_version < '3.11'
                                                               ^

Instead of the currently proposed:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/work/packaging/packaging/requirements.py", line 42, in __init__
    raise InvalidRequirement(str(e))
packaging.requirements.InvalidRequirement: Expected space before semicolon (followed by markers) or end of string
at position 58:
    tomli @ https://github.com/hukkin/tomli/archive/master.zip; python_version < '3.11'
                                                              ^

@uranusjr
Copy link
Copy Markdown
Member

uranusjr commented Sep 26, 2022

I think Pradyun means something like

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/work/packaging/packaging/requirements.py", line 42, in __init__
    raise InvalidRequirement(str(e))
packaging.requirements.InvalidRequirement: Expected space before semicolon (followed by markers) or end of string
at position 58:
    tomli @ https://github.com/hukkin/tomli/archive/master.zip; python_version < '3.11'

Maybe you mean this instead?
    tomli @ https://github.com/hukkin/tomli/archive/master.zip ; python_version < '3.11'

@Jackenmen
Add a hint per Pradyun's and Tzu-ping's suggestions
6d747a0 
Had to update `raise_syntax_error()` to accept a suffix string
to put after the source string and error position marker.
@Jackenmen Jackenmen mentioned this pull request Dec 3, 2022
Merged
@Jackenmen Jackenmen deleted the issue_529 branch January 5, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pradyunsg pradyunsg Awaiting requested review from pradyunsg

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Present a clearer error message when the user misses a space before ; in marker

4 participants

@Jackenmen @pradyunsg @uranusjr @brettcannon