packaging icon indicating copy to clipboard operation
packaging copied to clipboard

Fix erroneous non-ASCII in local version

Open zuo opened this issue 3 years ago • 9 comments

Closes: #469.

  • Fix validation of the given version in the packaging.version.Version's constructor.
  • Fix validation of the given spec in the packaging.specifiers.Specifier's constructor.
  • Complement relevant tests, including adding cases related to presence of non-ASCII whitespace characters in input strings.
  • Fix the docs of packaging.version.VERSION_PATTERN by mentioning necessity of the re.ASCII flag.

zuo avatar Oct 23 '21 23:10 zuo

@pradyunsg @brettcannon

Hmm, the error (some stringEnd vs. string_end discrepancy in an InvalidRequirement's exception message) that is reported by automatic checks seems unrelated to this PR.

=================================== FAILURES ===================================
________________ TestRequirements.test_parseexception_error_msg ________________

self = <tests.test_requirements.TestRequirements object at 0x00007f89be140fe0>

    def test_parseexception_error_msg(self):
        with pytest.raises(InvalidRequirement) as e:
            Requirement("toto 42")
>       assert "Expected stringEnd" in str(e.value)
E       assert 'Expected stringEnd' in 'Parse error at "\'42\'": Expected string_end'

zuo avatar Oct 26 '21 19:10 zuo

I think we also need tests to validate non-ASCII whitespaces are removed correctly.

uranusjr avatar Oct 26 '21 20:10 uranusjr

@uranusjr

I think we also need tests to validate non-ASCII whitespaces are removed correctly.

OK, added.

zuo avatar Oct 26 '21 22:10 zuo

I am sorry, I miss-clicked so that these 3 workflows become required again. :-(

zuo avatar Nov 13 '21 00:11 zuo

You didn't misclick -- you did all the right things. :)

It's just that GitHub requires me to click a button every time you modify a PR (It's their way to avoiding cryptomining endevours) to run our checks and I've done that now!

pradyunsg avatar Nov 13 '21 09:11 pradyunsg

Ah, ok. :-)

zuo avatar Nov 15 '21 00:11 zuo

What else do I need to do?

zuo avatar Nov 23 '21 14:11 zuo

What else do I need to do?

At this point probably just be patient. We are all busy and so it might be a while until someone has time to review this PR.

brettcannon avatar Nov 23 '21 22:11 brettcannon

@zuo I've got time to review this now (sorry about the delay 😅), but there are now merge conflicts. Did you want to update the PR?

brettcannon avatar Aug 19 '22 19:08 brettcannon