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

Jump to bottom

Disable universal newlines when reading TOML #10893

Merged
merged 6 commits into from Dec 1, 2021
Merged

Disable universal newlines when reading TOML #10893

merged 6 commits into from Dec 1, 2021

Conversation

hukkin
Copy link
Contributor

@hukkin hukkin commented Jul 30, 2021

Reading a file with universal newlines is not TOML compatible.

See e.g. toml-lang/toml#837 for context.

@JelleZijlstra JelleZijlstra self-assigned this Jul 30, 2021
@hauntsaninja hauntsaninja mentioned this pull request Aug 4, 2021
Closed
@hauntsaninja
Copy link
Collaborator

(I believe the current plan for this PR is to follow the lead of pypa/pip#10238 (comment) )

@JelleZijlstra
Copy link
Member

Yes. I assigned myself this PR and the corresponding one to Black and I plan to follow pip's decision.

@msullivan msullivan mentioned this pull request Aug 6, 2021
Merged
msullivan added a commit that referenced this pull request Aug 6, 2021
@msullivan
Pin tomli to 1.1.0
b223499 
While we figure out what the plan is in coordination with black and
pip (see #10893), just pin to the previous version of tomli that isn't
causing our CI to fail.
ethanhs pushed a commit that referenced this pull request Aug 6, 2021
@msullivan
Pin tomli to 1.1.0 (#10938)
daed963 
While we figure out what the plan is in coordination with black and
pip (see #10893), just pin to the previous version of tomli that isn't
causing our CI to fail.
@github-actions

This comment has been minimized.

Sign in to view
@hukkin hukkin mentioned this pull request Aug 9, 2021
Closed
@ethanhs
Copy link
Collaborator

ethanhs commented Oct 19, 2021

It would appear that pip is going to do tomli.loads(f.read()) while making sure the encoding is utf-8, I suppose we should probably do the same here.

@ethanhs ethanhs mentioned this pull request Oct 19, 2021
Closed
hukkin
hukkin commented Oct 19, 2021
View reviewed changes
Copy link
Contributor Author

@hukkin hukkin left a comment •
edited

Choose a reason for hiding this comment

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

Added "suggested changes" for the change @ethanhs mentioned, so if that's what maintainers want they can just click on those "commit suggestion" buttons.

EDIT: Note that my recommendation is still to not click those buttons as that is technically speaking an incorrect way to read TOML v1.0.0. Invalid TOML may be parsed without an error being raised.

mypy/config_parser.py Outdated Show resolved Hide resolved
mypy/modulefinder.py Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@ilevkivskyi
Copy link
Member

I am going to apply the changes to unblock the release (also it looks like this is what black is doing).

ilevkivskyi and others added 2 commits December 1, 2021 17:45
@ilevkivskyi @hukkin
Update mypy/modulefinder.py
5fee414 
Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>
@ilevkivskyi @hukkin
Update mypy/config_parser.py
98fea60 
Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>
@ilevkivskyi ilevkivskyi merged commit 02d6c7a into python:master Dec 1, 2021
@hukkin hukkin deleted the toml-universal-newlines branch December 1, 2021 19:06
ilevkivskyi added a commit that referenced this pull request Dec 1, 2021
@hukkin @ilevkivskyi
Disable universal newlines when reading TOML (#10893)
524b60e 
* Disable universal newlines when reading TOML

* Update mypy/modulefinder.py

Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>

* Update mypy/config_parser.py

Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
@hukkin @ilevkivskyi @tushar-deepsource
Disable universal newlines when reading TOML (python#10893)
05b2065 
* Disable universal newlines when reading TOML

* Update mypy/modulefinder.py

Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>

* Update mypy/config_parser.py

Co-authored-by: Taneli Hukkinen <3275109+hukkin@users.noreply.github.com>

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilevkivskyi ilevkivskyi ilevkivskyi approved these changes

Assignees

@JelleZijlstra JelleZijlstra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hukkin @hauntsaninja @JelleZijlstra @ethanhs @ilevkivskyi