hans added a comment.
I'm not really qualified to review the implementation details, but this seems
good as far as I can tell. A few comments, mostly style related.
================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:39
private string style = "file";
+ private bool formatOnSaveEnabled = false;
+ private string formatOnSaveFileExtensions =
----------------
Perhaps just `formatOnSave`, similar to `sortIncludes` above?
================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:46
+ {
+ // Use MemberwiseClone to copy value types
+ var clone = (OptionPageGrid)MemberwiseClone();
----------------
Ultra nit: end the sentence with a period.
================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:88
- [Category("LLVM/Clang")]
+ [Category("Format Options")]
[DisplayName("Style")]
----------------
What do you think about using "clang-format" for the category name?
================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:178
+
+ [Category("Format On Save")]
+ [DisplayName("Enable")]
----------------
Does this mean the "FormatOnSave" is not nested under the same category as the
other clang-format options?
================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:276
+
+ if (Vsix.IsDocumentDirty(document))
+ {
----------------
Perhaps return early if `!Vsix.IsDocumentDirty(document)` instead?
https://reviews.llvm.org/D29221
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
