<div dir="ltr">Jim, I think you and I have talked about this in the past.  Care to comment?</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 21, 2016 at 11:18 AM, Sean Callanan <span dir="ltr"><<a href="mailto:scallanan@apple.com" target="_blank">scallanan@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">I tend to agree with Zachary on the overall principle – and I would be willing to clang-format functions when I modify them.  I’m concerned about a specific class of functions, though.  Let’s say I have a function that has had lots of activity (I’m thinking of, for example, ParseType off in the DWARF parser).  Unfortunately, such functions tend to be the ones that benefit most from clang-format.<br><div><br></div><div>In such a function, there’s a lot of useful history available via svn blame that helps when fixing bugs.  My concern is that if someone clang-formats this function after applying the <i>k</i>th fix, suddenly I've lost convenient access to that history.  It’s only available with a fair amount of pain, and this pain increases as more fixes are applied because now I need to interleave the info before and after reformatting.</div><div><br></div><div>Would it be reasonable to mark such functions as “Don’t clang-format”?  That could be also interpreted as a “// TODO add comments so what this does is more understandable”</div><div><br></div><div>Sean</div><div><br><div><blockquote type="cite"><div><div class="h5"><div>On Jan 21, 2016, at 10:59 AM, Zachary Turner via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:</div><br></div></div><div><div><div class="h5"><div dir="ltr">Also this is the same standard that applies to the rest of LLVM.  clang-format your patches.  Just because we haven't been consistently following the rules until now doesn't mean we should continue to not follow the rules going forward.  This way eventually the codebase slowly converges towards a properly formatted one.  If clang-format does something that you think looks awkward with respect to the surrounding code (perhaps within a single logical block or whatever else) then just touch a line of code in the surrounding area so that clang-format will do it too. Since it only formats the differential, you have as much control as you need to produce something that a) is consistent with the rules, and b) doesn't look awkward with respect to surrounding code.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 21, 2016 at 10:11 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'm not sure I agree.  I don't think anything will be awkwardly formatted with regards to the rest of the file.  The biggest thing this is going to fix are whitespace at the end of lines, line breakign conventions, and space between function name and parentheses.<div><br></div><div>If we're not going to enforce a coding style, why have one at all?  clang-format enforces it.</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 21, 2016 at 8:41 AM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Glad to see clang-format getting some improvements.<br></div><div><br></div><div><br></div><div class="gmail_extra"></div></div><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 7, 2016 at 10:30 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">As far as I'm aware, this is the last major incompatibility between LLDB's style and clang-format's feature set.<div><br></div><div>I would appreciate it if more people could try it out with a few of their patches, and let me know if any LLDB style incompatibilities arise in the formatted code.</div><div><br></div><div>I would eventually like to move towards requiring that all patches be clang-formatted before committing to LLDB.  </div></div>
</blockquote></div><div class="gmail_extra"><br></div></div></div><div dir="ltr"><div class="gmail_extra">Question to the group on that last part.  I think if we have a large body of code that is just getting a few tweaks to a method, having the patch run through the formatter could lead to some pretty ugly code.  Imagine a few lines of a file awkwardly formatted related to the rest of the file.  Since we're not trying to reformat everything at once (which makes for difficult code traceability), and given there was a large code base to start with before LLDB was part of LLVM, I'm not sure we want a blanket statement that says it must go through clang-format.  (I personally would be fine with doing whole new functions and other logical blocks of code via clang-format when inserted into existing code, but I think it probably extreme when we're talking about new little sections within existing functions).</div><div class="gmail_extra"><br></div><div class="gmail_extra">Thoughts?</div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr">-Todd</div></div>
</div></div></blockquote></div></blockquote></div></div></div>
_______________________________________________<br>lldb-dev mailing list<br><a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br></div></blockquote></div><br></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">-Todd</div></div>
</div>