klimek added inline comments.
================
Comment at: include/clang/Format/Format.h:769
@@ -768,1 +768,3 @@
+/// \brief Returns the replacements corresponding to applying \p Replaces and
+/// cleaning up the code after that.
----------------
cleanupAroundReplacements sounds good.
================
Comment at: lib/Format/Format.cpp:1466
@@ -1464,1 +1465,3 @@
+ template <typename T>
+ tooling::Replacements process(T Callable, bool *IncompleteFormat) {
tooling::Replacements Result;
----------------
Nit: I think it's idiomatic to call this a "Callback".
================
Comment at: lib/Format/Format.cpp:2143
@@ +2142,3 @@
+
+ CodeProcessor Processor(Expanded, SourceMgr, ID, Ranges);
+ auto Callable =
----------------
This is unexpected: I'd not have expected the CodeProcessor to be used to keep
state apart from what's used during a Process() run. I think we'll need a
different name.
================
Comment at: lib/Format/Format.cpp:2176
@@ +2175,3 @@
+ bool *IncompleteFormat) -> tooling::Replacements {
+ Cleaner Cleaner(Processor);
+ return Cleaner.runCleanup(Tokens, AnnotatedLines);
----------------
The way the Processor is passed into the callbacks so they can call stuff on
the processor again makes me think this is really tightly coupled and should
use inheritance.
Alternatively, we might want to try to break up the CodeProcessor into a part
that just runs the callback and keeps the state for running the callback
around, and an interface that is used by the callbacks - I'll need to look more
closely into this though.
http://reviews.llvm.org/D18551
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
