<div dir="ltr">On Fri, Aug 30, 2013 at 6:08 PM, Nico Rieck <span dir="ltr"><<a href="mailto:nico.rieck@gmail.com" target="_blank">nico.rieck@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
  Another thing I forgot to ask. Since more C# code might be introduced in the future, do we really want to settle on the vertically verbose Allman style for braces?<br></blockquote><div><br></div><div>I personally couldn't care less ;) I used what VS does by default. I'm happy to use LLVM C++ style, as I then can use clang-format...</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
================<br>
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:139<br>
@@ +138,3 @@<br>
+            {<br>
+                throw new Exception(process.StandardError.ReadToEnd());<br>
+            }<br>
----------------<br>
</div><div>Manuel Klimek wrote:<br>
> Nico Rieck wrote:<br>
> > IIRC redirecting the standard streams and reading this way can become problematic if the internal buffer becomes full. It might be better to use the {Output,Error}DataReceived events to collect the output asynchronously into a StringBuffer.<br>


> I carefully crafted the order to make sure this cannot happen. I now added comments that explain why I think this is safe (it requires some knowledge about how clang-format works; I think the probability that this will change in the future is very low and I also think it simplifies the code here enough that it's worth it).<br>


</div>This works fine for stdout, but stderr is read after WaitForExit which will deadlock if the stderr buffer (4096 bytes) is filled. So as long as clang-format never writes this much to stderr this will work. (Though the alternative of reading stderr asynchronously and waiting on a resetevent after process exit isn't that much more difficult.)<br>

</blockquote><div><br></div><div>Ah, right, I completely ignored stderr - I don't expect this to matter much, so are you OK with a FIXME and addressing this in a later patch?</div></div></div></div>