[PATCH] D13549: Added new options to ClangFormat VSIX package.

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 13 07:28:00 PDT 2015


aaron.ballman added a comment.

Generally looks good to me, with a few small nits.


================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:57
@@ +56,3 @@
+            {
+                StandardValuesCollection svc = new StandardValuesCollection(values);
+                return svc;
----------------
Can we just return directly here instead of storing in a local?

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:82
@@ -42,1 +81,3 @@
+                     "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
+                     "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
                      "    configuration file.\n" +
----------------
Why is 'file' now lowercased?

================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:116
@@ +115,3 @@
+                // Check if string contains quotes.
+                if (s.IndexOf('\"') != -1)
+                    throw new NotSupportedException("Filename cannot contain quotes");
----------------
I see now why this is being used, but it took a lot of context from review comments to understand. Can you please put in a more descriptive comment about why we're checking for quotes, despite them being invalid filename characters on Windows?


http://reviews.llvm.org/D13549





More information about the cfe-commits mailing list