[PATCH] Add clang-format VS extension

[Manuel Klimek]

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:94
@@ +93,3 @@
+            {
+                IVsUIShell uiShell = GetService(typeof(SVsUIShell)) as IVsUIShell;
+                Guid id = Guid.Empty;
----------------
Nico Rieck wrote:
> If you're not checking the result for null, you might as well use a normal cast instead of as.
Done, here and elsewhere. I now use the normal cast everywhere where I don't check for null.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:122
@@ +121,3 @@
+            var style = GetStyle().Replace("\"", "\\\"");
+            process.StartInfo.Arguments = " -offset " + offset.ToString() +
+                                          " -length " + length.ToString() +
----------------
Nico Rieck wrote:
> You can leave out the .ToString() here.
Cool, done.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:139
@@ +138,3 @@
+            {
+                throw new Exception(process.StandardError.ReadToEnd());
+            }
----------------
Nico Rieck wrote:
> 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.
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).

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:150
@@ +149,3 @@
+            // The SVsTextManager is a service through which we can get the active view.
+            var textManager = Package.GetGlobalService(typeof(SVsTextManager)) as IVsTextManager;
+            IVsTextView textView;
----------------
Nico Rieck wrote:
> Same here regarding "as".
Done.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:167
@@ +166,3 @@
+        {
+            OptionPageGrid page = GetDialogPage(typeof(OptionPageGrid)) as OptionPageGrid;
+            return page.Style;
----------------
Nico Rieck wrote:
> Same here.
Done.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:174
@@ +173,3 @@
+            ITextDocument document;
+            if (view.TextBuffer.Properties.TryGetProperty<ITextDocument>(typeof(ITextDocument), out document))
+            {
----------------
Nico Rieck wrote:
> You can leave out the <ITextDocument> here.
Done.

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:70
@@ +69,3 @@
+                return;
+            var text = view.TextBuffer.CurrentSnapshot.GetText();
+            var start = view.Selection.Start.Position.GetContainingLine().Start.Position;
----------------
Nico Rieck wrote:
> You use var here instead of string, but not for casts or new where it makes more sense.
You're right. I went over the code and made my use of var vs. the type more consistent.

================
Comment at: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs:17
@@ +16,3 @@
+[assembly: AssemblyTrademark("")]
+[assembly: AssemblyCulture("")]   
+[assembly: ComVisible(false)]     
----------------
Nico Rieck wrote:
> Trailing whitespace.
Hopefully done.

================
Comment at: tools/clang-format-vs/ClangFormat/Properties/AssemblyInfo.cs:36
@@ +35,2 @@
+
+[assembly: InternalsVisibleTo("ClangFormat_UnitTests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f9ce6dec85f6d927d2f829d2c82bc3a467dd8ecf2397498ce8ac7ef839678553af1fc2ced523a0ea4394853ced10f1cbe31cbc7f29d4e98ab1513073410f114b8d679bbef548f656ca0c10859b9cf75a423908dcd6cedfc80b60c32ea20ae3352976ee4beb8c6c08e2eb82beb1e7ee6d7ad12189de1444edd25825f97b88f896")]
----------------
Kim Gräsman wrote:
> Are these tests available somewhere? Otherwise you could get rid of the InternalsVisibleTo attribute.
Yea, I deleted the tests :) They didn't seem to verify anything interesting, and I don't expect this code to change much (if at all).
Deleted the line now.


http://llvm-reviews.chandlerc.com/D1543