<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Mar 7, 2014 at 7:58 PM, Ahmed Charles <span dir="ltr"><<a href="mailto:acharles@outlook.com" target="_blank">acharles@outlook.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
  Thanks.<br>
<div class=""><br>
<br>
================<br>
Comment at: include/clang/Frontend/FrontendAction.h:149<br>
@@ -148,5 +148,3 @@<br>
<br>
-  ASTUnit *takeCurrentASTUnit() {<br>
-    return CurrentASTUnit.take();<br>
-  }<br>
+  ASTUnit *takeCurrentASTUnit() { return CurrentASTUnit.release(); }<br>
<br>
----------------<br>
</div><div class="">David Blaikie wrote:<br>
> I think clang-format might now have a mode in which it doesn't reformat lines you didn't edit (so this would remain on 3 lines instead of being clumped into one)...<br>
><br>
> But I'm not too fussed - just a possible FYI for this or next time in case you endu p dealing with more complicated patches that would benefit from easier diffs.<br>
</div>I don't see that as a command-line flag. It does refuse to make changes to certain things which are on lines that weren't edited, but it also does things like this. In any case, I went through each edit and decided they were fine given the complexity of the actual change going on and hopefully it will make future changes have nicer diffs.<br>

<div class=""><br>
================<br>
Comment at: lib/CodeGen/ModuleBuilder.cpp:51<br>
@@ -50,5 +50,3 @@<br>
<br>
-    virtual llvm::Module* ReleaseModule() {<br>
-      return M.take();<br>
-    }<br>
+    virtual llvm::Module *ReleaseModule() { return M.release(); }<br>
<br>
----------------<br>
</div><div class="">David Blaikie wrote:<br>
> Feel free to commit cleanup (like the moved '*' or the move to one-line - if you aren't using the "only modify lines I touched" thing I mentioned earlier - or if I hallucinated and that doesn't really exist) ahead of time without precommit review if you like - though it's not a big deal on a patch like this that's been manufactured by replacements and clang-format.<br>

</div>Interestingly enough, the first diff is without clang-format running and the second is after it was run, so you could always just look at the first one, which would have the same effect, at least for pre-review purposes.<br>

<br>
But agreed, that would be nicer for more complex changes.<br>
<div class=""><br>
================<br>
Comment at: lib/Lex/PTHLexer.cpp:539<br>
@@ -540,1 +538,3 @@<br>
+                        SL.release(), NumIds, spellingBase,<br>
+                        (const char *)originalSourceBase);<br>
 }<br>
----------------<br>
</div><div class="">David Blaikie wrote:<br>
> More orthogonal cleanup - might be worth filing bugs on clang-format-patch (or git-clang-format, or whatever other tools) for these.<br>
</div>I'm not sure whether it's a bug or by design. The vim integration code for clang-format says: The line or region is extended to the next bigger syntactic entity.<br>
<br>
This suggests that this is possibly a feature. I'm not sure what it would mean to reflow the first two lines without ever changing the third in this scenario, for example, if adding release make it hit the 80-col limit twice, it would obviously be better to change the third line rather than insert a new one. But in making that work correctly, you end up with orthogonal changes happening as well.<br>

<br>
In any case, it seems like clang-format isn't designed to simply ignore existing formatting, for example, there's no option to have it not touch the following code:<br>
<br>
`#include <foo> // comment`<br>
`#include <barbarbar> // comment`<br>
`#include <baz>       // comment`<br>
<br>
It will either change the indentation of the comment on line 1 or line 3, but no matter what you do, it will change one of them.<br></blockquote><div><br></div><div>clang-format should (if used with the right scripts) only re-format statements around touched lines (it will mostly re-format full statements though).</div>
<div> </div><div>What scripts did you use to clang-format the changes?</div><div><br></div><div>Cheers,</div><div>/Manuel</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D3000" target="_blank">http://llvm-reviews.chandlerc.com/D3000</a><br>
</blockquote></div><br></div></div>