-
-
Notifications
You must be signed in to change notification settings - Fork 433
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
open tomli file with newline='' #1196
Conversation
@@ -40,7 +40,7 @@ def read(self, filenames): | |||
filename = os.fspath(filenames) | |||
|
|||
try: | |||
with open(filename, encoding='utf-8') as fp: | |||
with open(filename, encoding='utf-8', newline='') as fp: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hukkin do you have a test file that breaks without newline=''
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any file that has a CR character not immediately followed by a LF character. The most simple example is a one byte file with just the CR character (0x0D).
That is invalid TOML and should raise an error, but doesn't if newline=''
is not set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll push a test case tomorrow
0c14f1a
to
82169a6
Compare
I know this is a draft, but is it still valid? |
Yes, this is still valid. |
@hukkin can you show me a toml file that causes a problem? Use
|
I don't think it's likely this will ever really cause a problem. But the "problem" is that the file you generated was parsed fine even though it is invalid TOML. A |
Thanks for the explanation. I don't see the need to enforce this kind of strictness, since no one will have a TOML file like this. |
No description provided.