<div dir="ltr">On Wed, Jan 23, 2013 at 12:04 PM, Philip Craig <span dir="ltr"><<a href="mailto:philipjcraig@gmail.com" target="_blank">philipjcraig@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
  Does this still need updating for the comment dumping? (and the color patch too)<br>
<br>
  I don't like how fiddly the lastChild() handling is, but I can't think of any alternative besides buffering the output, and I'm not sure doing that would be worth the effort.<br>
<br>
  Could you include more context in future patches please.<br>
<br>
  Manuel, can Phabricator be changed to display a separator when there are missing lines due to limited context?<br></blockquote><div><br></div><div style>Can you point me at where it doesn't? Also, in general, consider uploading patches with a higher context (see <a href="http://llvm.org/docs/Phabricator.html">http://llvm.org/docs/Phabricator.html</a>).</div>
<div style><br></div><div style>Cheers,</div><div style>/Manuel</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
<br>
================<br>
Comment at: lib/AST/ASTDumper.cpp:40<br>
@@ +39,3 @@<br>
+<br>
+    /// Indents - Indents[i] indicates if another child exists at level i.<br>
+    /// Used by Indent() to print the tree structure.<br>
----------------<br>
Don't put the name at the beginning of the comment.<br>
<br>
================<br>
Comment at: lib/AST/ASTDumper.cpp:541<br>
@@ -445,1 +540,3 @@<br>
+  setMoreChildren(HasDeclContext);<br>
+  if (HasAttrs) {<br>
     for (AttrVec::const_iterator I = D->getAttrs().begin(),<br>
----------------<br>
I think this test is some code I forgot to delete. The iterator check in the for loop is enough, so no need for this or the hasAttrs() call above.<br>
<br>
================<br>
Comment at: lib/AST/ASTDumper.cpp:549<br>
@@ -448,2 +548,3 @@<br>
   }<br>
   // Decls within functions are visited by the body<br>
+  setMoreChildren(false);<br>
----------------<br>
This comment might make more sense if it is moved up to HasDeclContext?<br>
<br>
================<br>
Comment at: lib/AST/ASTDumper.cpp:626<br>
@@ -519,3 +625,3 @@<br>
<br>
-  if (const FunctionTemplateSpecializationInfo *FTSI =<br>
-      D->getTemplateSpecializationInfo())<br>
+  bool oldMoreChildren = hasMoreChildren();<br>
+  const FunctionTemplateSpecializationInfo *FTSI =<br>
----------------<br>
Use upper case for variables. Elsewhere too.<br>
<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D281" target="_blank">http://llvm-reviews.chandlerc.com/D281</a><br>
</blockquote></div><br></div></div>