Merge newline token into break token. #118
Merged
Conversation
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
The handling of separate newlines and breaks in the PrettyPrinter resulted in a fairly complex implementation. In particular, it required storing the most recent break, and mutating the indentation stack to retroactively "fire" a break. This has been a source of bugs.
This updates the TokenStreamCreator and PrettyPrinter to a model without an explicit newline token. Instead, newlines are encapsulated by breaks. The same types of newlines exist ("flexible", "discretionary", and "mandatory" newlines have equivalent counterparts), but they exist as associated data with a break. This means newlines always exist where breaks are by definition.
This refactor "broke" some cases where comments were working with the right indentation "by accident". In cases where a comment existed outside of any scoping tokens (i.e. open/close breaks, continue breaks, etc.), the PrettyPrinter carried over indentation of the last break even if that break wasn't really relevant. To fix this, I added `splitScopingBeforeTokens(of:)` which splits the before-tokens so that those that start a scope can be placed before comments when visiting a token. This fixed several existing comment indentation issues, and fixed comments whose indentation was only "accidentally" working thanks to the PrettyPrinter's break carry-over behavior.
allevato
approved these changes
Jan 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The handling of separate newlines and breaks in the PrettyPrinter resulted in a fairly complex implementation. In particular, it required storing the most recent break, and mutating the indentation stack to retroactively "fire" a break. This has been a source of bugs.
This updates the TokenStreamCreator and PrettyPrinter to a model without an explicit newline token. Instead, newlines are encapsulated by breaks. The same types of newlines exist ("flexible", "discretionary", and "mandatory" newlines have equivalent counterparts), but they exist as associated data with a break. This means newlines always exist where breaks are by definition.
This refactor "broke" some cases where comments were working with the right indentation "by accident". In cases where a comment existed outside of any scoping tokens (i.e. open/close breaks, continue breaks, etc.), the PrettyPrinter carried over indentation of the last break even if that break wasn't really relevant. To fix this, I added
splitScopingBeforeTokens(of:)which splits the before-tokens so that those that start a scope can be placed before comments when visiting a token. This fixed several existing comment indentation issues, and fixed comments whose indentation was only "accidentally" working thanks to the PrettyPrinter's break carry-over behavior.