[PATCH] D18497: Auto-install LLVM Visual Studio visualizers for VS2015 and up

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

This is fantastic! Just a few nits, but nothing substantial.


================
Comment at: CMakeLists.txt:739
@@ +738,3 @@
+# tries to create a corresponding executable, which we don't want
+if(LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
+  set( LLVM_VISUALIZERS utils/llvm.natvis)
----------------
I think the convention is to have a space between the if and the (.

================
Comment at: CMakeLists.txt:740-741
@@ +739,4 @@
+if(LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
+  set( LLVM_VISUALIZERS utils/llvm.natvis)
+  add_custom_target( LLVMVisualizers SOURCES ${LLVM_VISUALIZERS})
+  set_target_properties(LLVMVisualizers PROPERTIES FOLDER "Utils")
----------------
Should remove the space after the open paren.

================
Comment at: CMakeLists.txt:781
@@ -766,2 +780,3 @@
 
+
 # This must be at the end of the LLVM root CMakeLists file because it must run
----------------
Spurious newline?

================
Comment at: utils/llvm.natvis:8
@@ -7,1 +7,3 @@
+
+For later versions of Visual Studio, no setup is required
 -->
----------------
Missing a terminating period.


http://reviews.llvm.org/D18497