<div dir="ltr">Maybe s/isExternCBlock/startsExternCBlock/.<div><br></div><div>Otherwise looks good. Thanks!</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 12, 2014 at 11:46 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</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">Now uses getNextNonComment().</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 12, 2014 at 2:26 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</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"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, Nov 12, 2014 at 1:46 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><div dir="ltr"><div style="font-family:arial,sans-serif;font-size:13px">Grml... Really prefer reviews on phabricator ...</div></div></span></blockquote><div><br></div></span><div>(I try to set up the command-line tool every now and then, it always ends up being a huge mess. And using the web interface takes 10x as long as just emailing the output of `svn diff` – I need to figure out how to convince svn how to include more context, click through a a bunch of web forms, etc. Since it's a small patch, I figured it's ok. If uploading to phab was easier, I'd use it more.)</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><div dir="ltr"><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">>+    } else if (Limit != 0 && Line.First->isNot(tok::kw_namespace) &&</div><div style="font-family:arial,sans-serif;font-size:13px">>+               (Line.First->isNot(tok::kw_extern) ||</div><div style="font-family:arial,sans-serif;font-size:13px">>+                (Line.First->Next && !Line.First->Next->isStringLiteral()))) {</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Isn't this doing something entirely different, namely not contracting such declaration to a single line. I'd be fine with that, but it should be called out explicitly.. And fixes part of <a href="http://llvm.org/PR21419" target="_blank">llvm.org/PR21419</a>.</div></div></span></blockquote><div><br></div></span><div>Ah, true. I only did what my description claimed at first, and then my testcase failed since it got formatted as a single line, so I figured this should probably be handled the same way namespaces are too. I'll call it out in the commit message.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><div dir="ltr"><div style="font-family:arial,sans-serif;font-size:13px">Also, this would prevent:<br></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">  extern "C" int f() { return 42; }</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Right? Probably not what we want.</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px"><div>>+        PreviousLine->First->isNot(tok::kw_namespace) &&</div><div>>+        PreviousLine->First->isNot(tok::kw_extern))</div></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">!PreviousLine->First->isOneOf(tok::kw_namespace, tok::kw_extern)</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Also, this prevents removing the empty line in</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">  extern "C" int f() {</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">    int i = 42;</div><div style="font-family:arial,sans-serif;font-size:13px">    return i;</div><div style="font-family:arial,sans-serif;font-size:13px">  }</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">Also not what we want, I guess.</div></div></span></blockquote><div><br></div></span><div>Great catch! Updated patch attached.</div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>