Skip to content
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

Ignore leading whitespace when checking for HTML doctype #10855

Closed
wants to merge 4 commits into from

Conversation

domdfcoding
Copy link
Contributor

Copy link

@malkir malkir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

src/pip/_internal/index/collector.py Outdated Show resolved Hide resolved
@domdfcoding
Copy link
Contributor Author

I'm not sure about the CI failures. They're getting AttributeError: 'int' object has no attribute 'startswith' from within setuptools.

@domdfcoding domdfcoding marked this pull request as ready for review January 31, 2022 14:29
@uranusjr
Copy link
Member

Hmm, I read the HTML5 spec and it seems we are still missing a lot of logic. This PR stands on its own so we should still merge it (unless you want to tackle them in this PR), but hopefully someone can come in and fix those other issues.

@notatallshaw
Copy link
Member

I'm not sure about the CI failures. They're getting AttributeError: 'int' object has no attribute 'startswith' from within setuptools.

Yes, seems tests are broken on Ubuntu at the moment, I am getting the same error for something completely unrelated: https://github.com/pypa/pip/runs/5010487348?check_suite_focus=true

@pradyunsg
Copy link
Member

I see @domdfcoding has filed pypa/setuptools#3063 already! :)

@pradyunsg pradyunsg dismissed pfmoore’s stale review January 31, 2022 18:55

This concern has been addressed!

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pfmoore
Copy link
Member

pfmoore commented Jan 31, 2022

hopefully someone can come in and fix those other issues.

I'm willing to take a look at this (I was intending to, in fact) but my time till the end of February is somewhat limited, so if someone else wants to get to it first that's fine.

@pradyunsg
Copy link
Member

In #10868, I'm proposing we drop this fallback I added in the spirit of expidiecy yesterday -- I think there's a much better behaviour here that I didn't think of in the heat of the moment yesterday.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants