Don't allow type comments to be merged behind regular comments#1027
Merged
Conversation
Type comments only apply if they are the first comment on the line, which means that allowing them to be pushed behind a regular comment when joining lines is a semantic change (and, indeed, one that black catches and fails on).
JelleZijlstra
approved these changes
Sep 17, 2019
Collaborator
|
Thanks! |
msullivan
added a commit
to msullivan/black
that referenced
this pull request
Oct 10, 2019
The code introduced in psf#1027 to detect whether a type comment appeared after a regular comment in a Line would spuriously misfire when a leaf was in the comments dict but had an empty list of comments. This can occur as an artifact of how comments on trailing commas are handled, it seems. (This was discovered trying to test black out on mypy.)
JelleZijlstra
pushed a commit
that referenced
this pull request
Oct 10, 2019
The code introduced in #1027 to detect whether a type comment appeared after a regular comment in a Line would spuriously misfire when a leaf was in the comments dict but had an empty list of comments. This can occur as an artifact of how comments on trailing commas are handled, it seems. (This was discovered trying to test black out on mypy.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type comments only apply if they are the first comment on the line,
which means that allowing them to be pushed behind a regular comment
when joining lines is a semantic change (and, indeed, one that black
catches and fails on).
(This issue discovered in the wild at Dropbox by a user who accidentally omitted a colon in a type comment and was confused to find black crashing.)