This is an archive of the discontinued LLVM Phabricator instance.

Refactor the formatter of clang-format.
ClosedPublic

Authored by klimek on May 8 2015, 1:35 AM.

Details

Reviewers
djasper
Summary

Pull various parts of the UnwrappedLineFormatter into their own abstractions.
NFC.

There are two things left for subsequent changes (to keep this reasonably small)

  • the UnwrappedLineFormatter now has a bad name
  • the UnwrappedLineFormatter::format function is still too large

The current plan is to rename the UnwrappedLineFormatter into BlockFormatter,
put more of the local variables of format() into class members (perhaps add
an abstraction for keeping track of the indent levels), and split format() up.

Diff Detail

Event Timeline

klimek updated this revision to Diff 25288.May 8 2015, 1:35 AM
klimek retitled this revision from to Refactor the formatter of clang-format..
klimek updated this object.
klimek edited the test plan for this revision. (Show Details)
klimek added a reviewer: djasper.
klimek added a subscriber: Unknown Object (MLST).
djasper added inline comments.May 8 2015, 5:24 AM
lib/Format/UnwrappedLineFormatter.cpp
458

I think this is a NoLineBreakFormatter. The fact that it is used if something fits into a line is not really a description of its function.

lib/Format/UnwrappedLineFormatter.h
40–41

I am not sure what the long term solution for UnwrappedLineFormatter/BlockFormatter is. I think we will probably want to try to separate the individual concepts into different classes (indent level tracking, deciding which line to format and how, deciding which formatter to use).

Until we have that, I'd like it to remain a container for the formatting configuration and pass it to the different LineFormatters (so that LineFormatter::formatChildren() can just call format).

klimek added inline comments.May 8 2015, 7:58 AM
lib/Format/UnwrappedLineFormatter.cpp
458

Done. Good name.

lib/Format/UnwrappedLineFormatter.h
40–41

Done.

klimek updated this revision to Diff 25331.May 8 2015, 7:58 AM

Apply review comments.

djasper accepted this revision.May 11 2015, 12:37 AM
djasper edited edge metadata.

Nice.

lib/Format/UnwrappedLineFormatter.h
42–44

I think 'const' isn't really necessary.

This revision is now accepted and ready to land.May 11 2015, 12:37 AM
klimek added inline comments.May 11 2015, 12:39 AM
lib/Format/UnwrappedLineFormatter.h
42–44

Done.

klimek closed this revision.Jul 2 2015, 6:46 AM