[PATCH] D55189: Extract TextNodeDumper class

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Basically looking good, modulo namespace questions from D55188 <https://reviews.llvm.org/D55188> and a few other organizational questions.



================
Comment at: include/clang/AST/ASTTextNodeDumper.h:1
+//===--- ASTTextNodeDumper.h - Printing of AST nodes ----------------------===//
+//
----------------
Perhaps the file should be named `TextNodeDumper.h` to match the class name?


================
Comment at: include/clang/AST/ASTTextNodeDumper.h:44
+
+  void dumpPointer(const void *Ptr) {
+    ColorScope Color(OS, ShowColors, AddressColor);
----------------
Should these implementations live in the header file? I feel like they belong in a .cpp file instead.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55189/new/

https://reviews.llvm.org/D55189