Join line comments tokens that belong to the same section together in the same unwrapped line.
Adapt ContinuationIndenter to pass more fields to the BreakableToken to make reflow possible.
Join BreakableBlockComment and BreakableLineComment into a single BreakableComment class.
Implement reflowing within BreakableComment::replaceWhitespaceBefore.
Add a bunch of comment reflowing tests.
Details
- Reviewers
klimek
Diff Detail
- Build Status
Buildable 2309 Build 2309: arc lint + arc unit
Event Timeline
Question is, do I split the newly introduced BreakableComment into two separate classes now (BreakableBlockComment and BreakableLineCommentSection), since there's not a whole lot of reflow reuse functionality -- we need to do totally different things in these two cases.
lib/Format/BreakableToken.cpp | ||
---|---|---|
457 | Without looking into this in a lot of detail: this looks like you want a BreakableComment base class, and have BreakableBlockComment and BreakableLineCommentSection derive from it and implement this method. Scanning it a bit, it seems like there is still overlap - perhaps it's also possible to pull out a couple of smaller sized methods in the interface and write the algorithm in terms of those? That could also make it easier to understand in general (large method alarm ;) | |
lib/Format/ContinuationIndenter.cpp | ||
1174–1176 | Nice that this whole section required so few changes. |
lib/Format/BreakableToken.cpp | ||
---|---|---|
457 | Next step I'm doing: will extract two subclasses and try to factor out the common functionality. I already did a bit of that with getReflowSplit, but there's more potential for extracting common stuff. | |
lib/Format/ContinuationIndenter.cpp | ||
1174–1176 | Yes, replaceWhitespaceBefore recomputes the ContentColumn in case a reflow with the previous line is decided to be made. |
lib/Format/ContinuationIndenter.cpp | ||
---|---|---|
1174–1176 | For my curiosity: what's the reason we can't precompute those in the constructor? Is that too much things we do outside of the flow here? |
- Split BreakableComment back into a BreakableBlockComment and BreakableLineCommentSection
- Extract common reflow computation functionality
- Put LastLineNeedsDecoration and Decoration and Prefix to the appropriate subclasses
- Fixed a double indentation bug caused by the WhitespaceManager.
Next 2 steps: will implement a simple heuristic about some sorts of ascii-art types of things; next will branch and produce a version that takes the reflow state out of the BreakableComment classes and puts it in the breakProtrudingToken implementation.
Ready for review.
- Split BreakableComment back into a BreakableBlockComment and BreakableLineCommentSection
- Extract common reflow computation functionality
- Put LastLineNeedsDecoration and Decoration and Prefix to the appropriate subclasses
- Fixed a double indentation bug caused by the WhitespaceManager.
- Implement a simple heuristic to detect ascii-art to stop reflow early
Without looking into this in a lot of detail: this looks like you want a BreakableComment base class, and have BreakableBlockComment and BreakableLineCommentSection derive from it and implement this method.
Scanning it a bit, it seems like there is still overlap - perhaps it's also possible to pull out a couple of smaller sized methods in the interface and write the algorithm in terms of those? That could also make it easier to understand in general (large method alarm ;)