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

Don't emit a trailng newline in base64-encoded data like 'image/png' #931

Merged
merged 2 commits into from
Feb 12, 2023

Conversation

xl0
Copy link
Contributor

@xl0 xl0 commented Feb 12, 2023

This should address #930

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blink1073 blink1073 added the bug label Feb 12, 2023
@blink1073 blink1073 enabled auto-merge (squash) February 12, 2023 15:30
@kevin-bates
Copy link
Member

@xl0 thanks for the pull request and analysis, it all makes sense to me.

My only concern, given the time that a training \n has been in use, is that some applications may be expecting it or will not tolerate its absence gracefully and i wonder if this is something that should be configurable, at least initially.

I'm unable to check the history of that call but I would imagine it's been around since prior to python 3.6.

This configurable could be something akin to the message debug option where, iirc, it isn't a configurable trait. That would at least give intolerant applications a way forward.

(Sorry, I've access to only my phone at the moment but wanted to raise this topic.)

@blink1073
Copy link
Contributor

I would say anyone consuming these values would have most likely called strip() on them, which would still work.

@blink1073 blink1073 merged commit e77ad89 into jupyter:main Feb 12, 2023
@xl0 xl0 mentioned this pull request Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants