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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to