Skip to content

bpo-38870: Don't start generated output with newlines#19636

Merged
pablogsal merged 2 commits intopython:masterfrom
isidentical:bpo-38870-newline
May 3, 2020
Merged

bpo-38870: Don't start generated output with newlines#19636
pablogsal merged 2 commits intopython:masterfrom
isidentical:bpo-38870-newline

Conversation

@isidentical
Copy link
Copy Markdown
Member

@isidentical isidentical commented Apr 21, 2020 •
edited by bedevere-bot
Loading

This is a small nit that can cause too much unnecessary nose when using ast.unparse for comparisons. Examples

 $ python
Python 3.9.0a5+ (heads/master:5565c30f0b, Apr 17 2020, 16:56:39) 
[GCC 7.5.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ast
>>> ast.unparse(ast.parse("compile"))
'\ncompile'
>>> ast.unparse(ast.parse("compile")) == "compile"
False
>>> ast.unparse(ast.parse("class compile: pass"))
'\n\nclass compile():\n    pass'

https://bugs.python.org/issue38870

@isidentical isidentical requested a review from pablogsal April 21, 2020 22:40
pablogsal
pablogsal approved these changes Apr 27, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

Just a nit: I find newline() a bit misleading if it can actually not produce a newline. Maybe rename to maybe_newline or something like that. Otherwise LGTM

@isidentical
Copy link
Copy Markdown
Member Author

isidentical commented May 3, 2020

@pablogsal okey, I wasn't sure if we wanted to expose that in the method name but changed it to maybe_newline anyways.

@pablogsal pablogsal merged commit 493bf1c into python:master May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@isidentical @pablogsal @the-knights-who-say-ni @bedevere-bot