[PATCH] D18551: Added Fixer implementation and fix() interface in clang-format for removing redundant code.

[Eric Liu via cfe-commits]

ioeric added inline comments.

================
Comment at: lib/Format/Format.cpp:1596
@@ +1595,3 @@
+
+class Fixer : public UnwrappedLineConsumer {
+public:
----------------
djasper wrote:
> I am not sure, this is the right class to pull out. It still has a lot of overlap with formatter. Maybe it is instead a better idea to pull out each fixer into its own class and pull them into the formatter.
But I'm not quite sure if implementing the fixer in formatter works better since they look like two parallel classes to me. And it might make the formatter more complicated to use if we mix it with fixer. I think adding another layer of abstraction might be one way to reduce duplication? 

================
Comment at: lib/Format/Format.cpp:1654
@@ +1653,3 @@
+
+    while (CurrentToken->isNot(tok::eof)) {
+      switch (CurrentToken->Tok.getKind()) {
----------------
djasper wrote:
> I am not (yet) convinced that the fundamental design here makes sense. So far it seems to me that all fixers will either operate within a single line (ctor fixer and probably most others) or that they do something specific on multiple lines (namespace fixer and empty public/private section fixer and maybe others). Creating a new way to iterate over all tokens of all lines doesn't really add useful functionality here AFAICT.
I've also considered the way formatter does, which iterates over lines. But it seems to be easier to write fixer, especially multi-line fixer, if we can just ignore lines. Working with lines seems to add another layer of complexity here?


http://reviews.llvm.org/D18551