clang-format-vsix: "format on save" feature

[amaiorano]

Summary

This change adds a feature to the clang-format VS extension that optionally enables the automatic formatting of documents when saving. Since developers always need to save their files, this eases the workflow of making sure source files are properly formatted. This feature exists in other IDEs, notably in Eclipse; furthermore, there is a VS extension that provides this functionality for generic formatters, so there is definitely a precedent for this type of feature.

Although this patch works, I know it's quite big and potentially contentious, so I'd like to start a discussion about it here. I would appreciate it if you could try it out locally for a couple days to see how it feels. Personally, I've been using it and like how it eases my workflow.

Here's a screenshot of what the options grid looks like now by default:

Things to note:

• I renamed the category "LLVM/Clang" to "Format Options" and added one to group the "Format on Save" options.

• The "File extensions" field is needed to filter out non-source code documents that are modified in VS, but that we don't want to format with clang-format. The list of extensions includes all possible C/C++ file extensions (I hope), and the list of extensions presently supported by clang-format.

• The "Mode" field allows you to select "All Documents" or "Current Document", which effectively scopes the formatting on save to all modified documents or current modified document respectively. I'm still on the fence on whether to even bother offering this mode, especially since "Current Document" can be weird since you could modify multiple files, then "save all" (which happens when you trigger a build), and find that only the current document gets formatted. Perhaps it's just better to only support "All Documents" and remove this field.

Other things I'd like to call out in this change:

• When formatting on save, we ignore the FallbackStyle in the user's options and set it to "none". This is to make sure we only format files based on Style: for e.g., if Style if "file", we only format on save when a .clang-format file is found; but we don't format files in projects that have no .clang-format. I think this makes sense since "format on save" is a global option, and users would be surprised to find it formatting files in projects that don't have a .clang-format file. The description of the "Enable" field in the options dialog explains this.

• I split out some helper functions into a VsixUtils class and file, and added new utility functions for format on save. I also added 2 new files with more related helpers.

• I added the attribute [ProvideAutoLoad(UIContextGuids80.SolutionExists)] to make the extension load as soon as a solution is loaded, rather than have it load lazily upon the first format line/document call. This is required so that I can register for the BeforeSave callback right away, otherwise formatting on save would not work until the user first explicitly formats (via menu or keyboard shortcut).

• I did some refactoring around OptionsPageGrid so that rather than accessing it directly in RunClangFormat(), we pass it down as args. This allows me to override user options, specifically the FallbackStyle, when formatting on save. (line 320)

Thanks, and I looked forward to your feedback!

[amaiorano]

Hello! Just a small ping to see if anyone has taken a look at this change? I fully understand that everyone's really busy, but perhaps you can recommend another reviewer? Or if you're presently giving this change a whirl, just let me know! Cheers :)

[klimek]

+hans

+1 to "format only current document but save all" not making much sense :)

tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs

7

Perhaps name this "TypeConversion" or something - I find "utils" as part of a name to not add anything, and it tends to accumulate cruft :)

tools/clang-format-vs/ClangFormat/VsixUtils.cs

13

Same here, just call it Vsix? (context seems clear enough)

[amaiorano]

In D29221#668867, @klimek wrote:

+hans

+1 to "format only current document but save all" not making much sense :)

Yes, I've been using this version for a little while now, and it's not useful to have the current document version. I'll remove it, which will simplify the code as well.

I'll wait a bit more for comments from others before uploading a new version.

Thanks!

tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs

7

Sounds great, will do that.

tools/clang-format-vs/ClangFormat/VsixUtils.cs

13

Perfect, will also make this change.

[amaiorano]

• Renamed VsixUtils to Vsix and TypeConverterUtils to TypeConversion
• Removed format on save mode

The change is simpler now without the "mode" (current or all documents). Now I'm wondering whether I should rename the feature to Format _Document_ On Save, and reflect this name at least in the options grid, if not the related variable names.

[amaiorano]

Hello again. Just wondering if anyone could take a look at this? I updated the patch a week ago :) Thanks!

[hans]

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.

tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs

45

Perhaps just formatOnSave, similar to sortIncludes above?

52

Ultra nit: end the sentence with a period.

95–102

What do you think about using "clang-format" for the category name?

192

Does this mean the "FormatOnSave" is not nested under the same category as the other clang-format options?

342

Perhaps return early if !Vsix.IsDocumentDirty(document) instead?

[amaiorano]

tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs

45

Will do.

52

Will do.

95–102

So if you take a look at the screenshot I posted with the original diff, you'll see how these categories show up:
https://reviews.llvm.org/file/data/cuztal767fqmcy2k7kkv/PHID-FILE-xcoqfwj3o2tpwbabbak5/pasted_file

"LLVM/Clang" is the main option menu name, and was always there. I figure the idea is that if we write other extensions, they'd all fall under this heading. Personally I'd like for it to be "clang-format" or "Clang Format".

As part of my change, I grouped the options that were there before under "Format Options", which is what they are (arguments to clang-format), and my new options under "Format On Save".

192

See my answer to the comment on line 88.

[amaiorano]

tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs

342

Yes, will do.

[amaiorano]

Made changes based on @hans 's feedback.

I looked again at the categories, and I think it makes sense with my changes. Here's an updated screenshot that shows what the options menu in Visual Studio looks like with these changes with the top-level category name ("LLVM/Clang") and page name ("Clang Format") highlighted, as well as the category names for the page items ("Format Options" and "Format On Save") highlighted:

It would be nice if "Format On Save" was _below_ "Format Options", but Visual Studio always orders the page categories in alphabetical order, so this is pretty standard.

[hans]

In D29221#686865, @amaiorano wrote:

Made changes based on @hans 's feedback.

I looked again at the categories, and I think it makes sense with my changes. Here's an updated screenshot that shows what the options menu in Visual Studio looks like with these changes with the top-level category name ("LLVM/Clang") and page name ("Clang Format") highlighted, as well as the category names for the page items ("Format Options" and "Format On Save") highlighted:

It would be nice if "Format On Save" was _below_ "Format Options", but Visual Studio always orders the page categories in alphabetical order, so this is pretty standard.

The screenshot looks good.

My only nit is that I'd prefer "clang-format" instead of "ClangFormat".

Manuel: the menu options under Tools currently say "Clang Format {Selection,Document}". What do you think about using "clang-format" here instead? That is the name of the tool after all, and I think it also works nicely as a verb.

I realize the actual extension is called ClangFormat, but maybe there were reasons for that, or we have to live with it?

[amaiorano]

In D29221#687425, @hans wrote:

In D29221#686865, @amaiorano wrote:

Made changes based on @hans 's feedback.

I looked again at the categories, and I think it makes sense with my changes. Here's an updated screenshot that shows what the options menu in Visual Studio looks like with these changes with the top-level category name ("LLVM/Clang") and page name ("Clang Format") highlighted, as well as the category names for the page items ("Format Options" and "Format On Save") highlighted:

It would be nice if "Format On Save" was _below_ "Format Options", but Visual Studio always orders the page categories in alphabetical order, so this is pretty standard.

The screenshot looks good.

My only nit is that I'd prefer "clang-format" instead of "ClangFormat".

Manuel: the menu options under Tools currently say "Clang Format {Selection,Document}". What do you think about using "clang-format" here instead? That is the name of the tool after all, and I think it also works nicely as a verb.

I realize the actual extension is called ClangFormat, but maybe there were reasons for that, or we have to live with it?

I would also like clang-format in there rather than ClangFormat. One thing to validate is whether this change would mean people would lose their changes to the defaults in the configuration panel. I'll run some tests and see if this is indeed the case. Maybe there's a way to keep the internal name the same for config purposes, but change the displayed name instead. Will get back to you.

[amaiorano]

In D29221#687438, @amaiorano wrote:

In D29221#687425, @hans wrote:

In D29221#686865, @amaiorano wrote:

Made changes based on @hans 's feedback.

I looked again at the categories, and I think it makes sense with my changes. Here's an updated screenshot that shows what the options menu in Visual Studio looks like with these changes with the top-level category name ("LLVM/Clang") and page name ("Clang Format") highlighted, as well as the category names for the page items ("Format Options" and "Format On Save") highlighted:

It would be nice if "Format On Save" was _below_ "Format Options", but Visual Studio always orders the page categories in alphabetical order, so this is pretty standard.

The screenshot looks good.

My only nit is that I'd prefer "clang-format" instead of "ClangFormat".

Manuel: the menu options under Tools currently say "Clang Format {Selection,Document}". What do you think about using "clang-format" here instead? That is the name of the tool after all, and I think it also works nicely as a verb.

I realize the actual extension is called ClangFormat, but maybe there were reasons for that, or we have to live with it?

I would also like clang-format in there rather than ClangFormat. One thing to validate is whether this change would mean people would lose their changes to the defaults in the configuration panel. I'll run some tests and see if this is indeed the case. Maybe there's a way to keep the internal name the same for config purposes, but change the displayed name instead. Will get back to you.

Okay, I looked into it, and the good news is that changing the name of the category from ClangFormat to clang-format doesn't reset the previously saved changes made in the options menu. I also changed the menu item names. Here's what both would look like:

Now having said that, I'm not sure if this is the best change because of a few things:

1. The extension's name itself is ClangFormat, which was recently added to the Visual Studio market place here: https://marketplace.visualstudio.com/items?itemName=HansWennborg.ClangFormat . Consequently, the extension appears with this name in the Visual Studio extensions dialog:
   
   . I don't think it would be easy to change this. You cannot really take down an extension page on the marketplace once it's there; you'd have to document that it has been deprecated and provide a link to a new page. When searching for it in the Extensions dialog in VS, though, both the old and new extension would show up.

2. Although the name of the executable is indeed "clang-format", the documentation here uses the name ClangFormat:
   

So I leave it up to you whether you really want this change or not. We can also decide later rather than fold it into this change.

[hans]

My only nit is that I'd prefer "clang-format" instead of "ClangFormat".

Manuel: the menu options under Tools currently say "Clang Format {Selection,Document}". What do you think about using "clang-format" here instead? That is the name of the tool after all, and I think it also works nicely as a verb.

I realize the actual extension is called ClangFormat, but maybe there were reasons for that, or we have to live with it?

I would also like clang-format in there rather than ClangFormat. One thing to validate is whether this change would mean people would lose their changes to the defaults in the configuration panel. I'll run some tests and see if this is indeed the case. Maybe there's a way to keep the internal name the same for config purposes, but change the displayed name instead. Will get back to you.

Okay, I looked into it, and the good news is that changing the name of the category from ClangFormat to clang-format doesn't reset the previously saved changes made in the options menu. I also changed the menu item names. Here's what both would look like:

Now having said that, I'm not sure if this is the best change because of a few things:

1. The extension's name itself is ClangFormat, which was recently added to the Visual Studio market place here: https://marketplace.visualstudio.com/items?itemName=HansWennborg.ClangFormat . Consequently, the extension appears with this name in the Visual Studio extensions dialog:
   
   . I don't think it would be easy to change this. You cannot really take down an extension page on the marketplace once it's there; you'd have to document that it has been deprecated and provide a link to a new page. When searching for it in the Extensions dialog in VS, though, both the old and new extension would show up.

2. Although the name of the executable is indeed "clang-format", the documentation here uses the name ClangFormat:
   

So I leave it up to you whether you really want this change or not. We can also decide later rather than fold it into this change.

Personally I think we should refer to the tool and the action of formatting as "clang-format" as much as possible. It's unfortunate we can't rename the extension, but maybe that slight inconsistency isn't the end of the world.

Manuel, Daniel: I'd like to hear your opinions here.

[amaiorano]

In D29221#688697, @hans wrote:

My only nit is that I'd prefer "clang-format" instead of "ClangFormat".

Manuel: the menu options under Tools currently say "Clang Format {Selection,Document}". What do you think about using "clang-format" here instead? That is the name of the tool after all, and I think it also works nicely as a verb.

I realize the actual extension is called ClangFormat, but maybe there were reasons for that, or we have to live with it?

I would also like clang-format in there rather than ClangFormat. One thing to validate is whether this change would mean people would lose their changes to the defaults in the configuration panel. I'll run some tests and see if this is indeed the case. Maybe there's a way to keep the internal name the same for config purposes, but change the displayed name instead. Will get back to you.

Okay, I looked into it, and the good news is that changing the name of the category from ClangFormat to clang-format doesn't reset the previously saved changes made in the options menu. I also changed the menu item names. Here's what both would look like:

Now having said that, I'm not sure if this is the best change because of a few things:

1. The extension's name itself is ClangFormat, which was recently added to the Visual Studio market place here: https://marketplace.visualstudio.com/items?itemName=HansWennborg.ClangFormat . Consequently, the extension appears with this name in the Visual Studio extensions dialog:
   
   . I don't think it would be easy to change this. You cannot really take down an extension page on the marketplace once it's there; you'd have to document that it has been deprecated and provide a link to a new page. When searching for it in the Extensions dialog in VS, though, both the old and new extension would show up.

2. Although the name of the executable is indeed "clang-format", the documentation here uses the name ClangFormat:
   

So I leave it up to you whether you really want this change or not. We can also decide later rather than fold it into this change.

Personally I think we should refer to the tool and the action of formatting as "clang-format" as much as possible. It's unfortunate we can't rename the extension, but maybe that slight inconsistency isn't the end of the world.

Manuel, Daniel: I'd like to hear your opinions here.

Hey guys, any feedback on this? About 1.5 weeks ago, @hans asked @klimek and @djasper for their opinions on "clang-format" vs "ClangFormat". Thanks!

[hans]

In D29221#698679, @amaiorano wrote:

Hey guys, any feedback on this? About 1.5 weeks ago, @hans asked @klimek and @djasper for their opinions on "clang-format" vs "ClangFormat". Thanks!

Let's get this committed now and not worry about renaming things.

Do you have commit access, or would you like me to commit it for you?

[amaiorano]

In D29221#713902, @hans wrote:

In D29221#698679, @amaiorano wrote:

Hey guys, any feedback on this? About 1.5 weeks ago, @hans asked @klimek and @djasper for their opinions on "clang-format" vs "ClangFormat". Thanks!

Let's get this committed now and not worry about renaming things.

Do you have commit access, or would you like me to commit it for you?

I've got commit access, so I'll commit this ASAP. Thanks!

tools/clang-format-vs/ClangFormat/ClangFormat.csproj

<Compile Include="Resources.Designer.cs">
       <DependentUpon>Resources.resx</DependentUpon>
     </Compile>
     <Compile Include="GlobalSuppressions.cs" />
     <Compile Include="ClangFormatPackage.cs">
       <SubType>Component</SubType>
     </Compile>
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="PkgCmdID.cs" />
+    <Compile Include="RunningDocTableEventsDispatcher.cs" />
+    <Compile Include="TypeConverterUtils.cs" />
+    <Compile Include="VsixUtils.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="Resources.resx">
       <Generator>ResXFileCodeGenerator</Generator>
       <LastGenOutput>Resources.Designer.cs</LastGenOutput>
       <SubType>Designer</SubType>
     </EmbeddedResource>
     <EmbeddedResource Include="VSPackage.resx">
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="4.0">
   <!-- To modify your build process, add your task inside one of the targets below and uncomment it. 
        Other similar extension points exist, see Microsoft.Common.targets.
   <Target Name="BeforeBuild">
   </Target>
   <Target Name="AfterBuild">
   </Target>
   -->
 </Project>
 No newline at end of file

tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs

 //===-- ClangFormatPackages.cs - VSPackage for clang-format ------*- C# -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This class contains a VS extension package that runs clang-format over a
 // selection in a VS text editor.
 //
 //===----------------------------------------------------------------------===//
 
-using Microsoft.VisualStudio.Editor;
+using EnvDTE;
 using Microsoft.VisualStudio.Shell;
 using Microsoft.VisualStudio.Shell.Interop;
 using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Text.Editor;
-using Microsoft.VisualStudio.TextManager.Interop;
 using System;
 using System.Collections;
 using System.ComponentModel;
 using System.ComponentModel.Design;
 using System.IO;
 using System.Runtime.InteropServices;
 using System.Xml.Linq;
+using System.Linq;
 
 namespace LLVM.ClangFormat
 {
+    public enum FormatOnSaveMode
+    {
+        AllDocuments,
+        CurrentDocument
+    }
+
     [ClassInterface(ClassInterfaceType.AutoDual)]
     [CLSCompliant(false), ComVisible(true)]
     public class OptionPageGrid : DialogPage
     {
         private string assumeFilename = "";
         private string fallbackStyle = "LLVM";
         private bool sortIncludes = false;
         private string style = "file";
+        private bool formatOnSaveEnabled = false;
+        private FormatOnSaveMode formatOnSaveMode = FormatOnSaveMode.AllDocuments;
+        private string formatOnSaveFileExtensions =
+            ".c;.cpp;.cxx;.cc;.tli;.tlh;.h;.hh;.hpp;.hxx;.hh;.inl" +
+            ".java;.js;.ts;.m;.mm;.proto;.protodevel;.td";
+
+        public OptionPageGrid Clone()
+        {
+            // Use MemberwiseClone to copy value types
+            var clone = (OptionPageGrid)MemberwiseClone();
+            return clone;
+        }
 
         public class StyleConverter : TypeConverter
         {
             protected ArrayList values;
             public StyleConverter()
             {
                 // Initializes the standard values list with defaults.
                 values = new ArrayList(new string[] { "file", "Chromium", "Google", "LLVM", "Mozilla", "WebKit" });
public class OptionPageGrid : DialogPage
                 string s = value as string;
                 if (s == null)
                     return base.ConvertFrom(context, culture, value);
 
                 return value;
             }
         }
 
-        [Category("LLVM/Clang")]
+        public class FormatOnSaveModeConverter : TypeConverterUtils.EnumStringTypeConverter
+        {
+            public FormatOnSaveModeConverter() : base(typeof(FormatOnSaveMode))
+            {
+            }
+        }
+
+        [Category("Format Options")]
         [DisplayName("Style")]
         [Description("Coding style, currently supports:\n" +
                      "  - Predefined styles ('LLVM', 'Google', 'Chromium', 'Mozilla', 'WebKit').\n" +
                      "  - 'file' to search for a YAML .clang-format or _clang-format\n" +
                      "    configuration file.\n" +
                      "  - A YAML configuration snippet.\n\n" +
                      "'File':\n" +
                      "  Searches for a .clang-format or _clang-format configuration file\n" +
public class OptionPageGrid : DialogPage
                 // A quote in user input would end the parameter quote and so break the command invocation.
                 if (s.IndexOf('\"') != -1)
                     throw new NotSupportedException("Filename cannot contain quotes");
 
                 return value;
             }
         }
 
-        [Category("LLVM/Clang")]
+        [Category("Format Options")]
         [DisplayName("Assume Filename")]
         [Description("When reading from stdin, clang-format assumes this " +
                      "filename to look for a style config file (with 'file' style) " +
                      "and to determine the language.")]
         [TypeConverter(typeof(FilenameConverter))]
         public string AssumeFilename
         {
             get { return assumeFilename; }
             set { assumeFilename = value; }
         }
 
         public sealed class FallbackStyleConverter : StyleConverter
         {
             public FallbackStyleConverter()
             {
                 // Add "none" to the list of styles.
                 values.Insert(0, "none");
             }
         }
 
-        [Category("LLVM/Clang")]
+        [Category("Format Options")]
         [DisplayName("Fallback Style")]
         [Description("The name of the predefined style used as a fallback in case clang-format " +
                      "is invoked with 'file' style, but can not find the configuration file.\n" +
                      "Use 'none' fallback style to skip formatting.")]
         [TypeConverter(typeof(FallbackStyleConverter))]
         public string FallbackStyle
         {
             get { return fallbackStyle; }
             set { fallbackStyle = value; }
         }
 
-        [Category("LLVM/Clang")]
+        [Category("Format Options")]
         [DisplayName("Sort includes")]
         [Description("Sort touched include lines.\n\n" +
                      "See also: http://clang.llvm.org/docs/ClangFormat.html.")]
         public bool SortIncludes
         {
             get { return sortIncludes; }
             set { sortIncludes = value; }
         }
+
+        [Category("Format On Save")]
+        [DisplayName("Enable")]
+        [Description("Enable running clang-format when modified files are saved. " +
+                     "Will only format if Style is found (ignores Fallback Style)."
+            )]
+        public bool FormatOnSaveEnabled
+        {
+            get { return formatOnSaveEnabled; }
+            set { formatOnSaveEnabled = value; }
+        }
+
+        [Category("Format On Save")]
+        [DisplayName("Mode")]
+        [Description("How to run clang-format upon saving:\n" +
+                     "- All Documents: format all modified documents\n" +
+                     "- Current Document: format current document, if modified")]
+        [TypeConverter(typeof(FormatOnSaveModeConverter))]
+        public FormatOnSaveMode FormatOnSaveMode
+        {
+            get { return formatOnSaveMode; }
+            set { formatOnSaveMode = value; }
+        }
+
+        [Category("Format On Save")]
+        [DisplayName("File extensions")]
+        [Description("When formatting on save, clang-format will be applied only to " +
+                     "files with these extensions.")]
+        public string FormatOnSaveFileExtensions
+        {
+            get { return formatOnSaveFileExtensions; }
+            set { formatOnSaveFileExtensions = value; }
+        }
     }
 
     [PackageRegistration(UseManagedResourcesOnly = true)]
     [InstalledProductRegistration("#110", "#112", "1.0", IconResourceID = 400)]
     [ProvideMenuResource("Menus.ctmenu", 1)]
+    [ProvideAutoLoad(UIContextGuids80.SolutionExists)] // Load package on solution load
     [Guid(GuidList.guidClangFormatPkgString)]
     [ProvideOptionPage(typeof(OptionPageGrid), "LLVM/Clang", "ClangFormat", 0, 0, true)]
     public sealed class ClangFormatPackage : Package
     {
         #region Package Members
+
+        RunningDocTableEventsDispatcher _runningDocTableEventsDispatcher;
+
         protected override void Initialize()
         {
             base.Initialize();
 
+            _runningDocTableEventsDispatcher = new RunningDocTableEventsDispatcher(this);
+            _runningDocTableEventsDispatcher.BeforeSave += OnBeforeSave;
+
             var commandService = GetService(typeof(IMenuCommandService)) as OleMenuCommandService;
             if (commandService != null)
             {
                 {
                     var menuCommandID = new CommandID(GuidList.guidClangFormatCmdSet, (int)PkgCmdIDList.cmdidClangFormatSelection);
                     var menuItem = new MenuCommand(MenuItemCallback, menuCommandID);
                     commandService.AddCommand(menuItem);
                 }
 
                 {
                     var menuCommandID = new CommandID(GuidList.guidClangFormatCmdSet, (int)PkgCmdIDList.cmdidClangFormatDocument);
                     var menuItem = new MenuCommand(MenuItemCallback, menuCommandID);
                     commandService.AddCommand(menuItem);
                 }
             }
         }
         #endregion
 
+        OptionPageGrid GetUserOptions()
+        {
+            return (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+        }
+
         private void MenuItemCallback(object sender, EventArgs args)
         {
             var mc = sender as System.ComponentModel.Design.MenuCommand;
             if (mc == null)
                 return;
 
             switch (mc.CommandID.ID)
             {
                 case (int)PkgCmdIDList.cmdidClangFormatSelection:
-                    FormatSelection();
+                    FormatSelection(GetUserOptions());
                     break;
 
                 case (int)PkgCmdIDList.cmdidClangFormatDocument:
-                    FormatDocument();
+                    FormatDocument(GetUserOptions());
                     break;
             }
         }
 
+        private static bool FileHasExtension(string filePath, string fileExtensions)
+        {
+            var extensions = fileExtensions.ToLower().Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
+            return extensions.Contains(Path.GetExtension(filePath).ToLower());
+        }
+
+        private void OnBeforeSave(object sender, Document document)
+        {
+            var options = GetUserOptions();
+
+            if (!options.FormatOnSaveEnabled)
+                return;
+
+            if (!FileHasExtension(document.FullName, options.FormatOnSaveFileExtensions))
+                return;
+
+            switch (options.FormatOnSaveMode)
+            {
+                case FormatOnSaveMode.CurrentDocument:
+                    IWpfTextView view = VsixUtils.GetCurrentView();
+                    if (view == null)
+                        // We're not in a text view.
+                        return;
+                    if (VsixUtils.GetDocumentView(document) != view)
+                        // Not the current document
+                        return;
+                    break;
+
+                case FormatOnSaveMode.AllDocuments:
+                    break;
+            }
+
+            if (VsixUtils.IsDocumentDirty(document))
+            {
+                var optionsWithNoFallbackStyle = GetUserOptions().Clone();
+                optionsWithNoFallbackStyle.FallbackStyle = "none";
+                FormatDocument(document, optionsWithNoFallbackStyle);
+            }
+        }
+
         /// <summary>
         /// Runs clang-format on the current selection
         /// </summary>
-        private void FormatSelection()
+        private void FormatSelection(OptionPageGrid options)
         {
-            IWpfTextView view = GetCurrentView();
+            IWpfTextView view = VsixUtils.GetCurrentView();
             if (view == null)
                 // We're not in a text view.
                 return;
             string text = view.TextBuffer.CurrentSnapshot.GetText();
             int start = view.Selection.Start.Position.GetContainingLine().Start.Position;
             int end = view.Selection.End.Position.GetContainingLine().End.Position;
             int length = end - start;
             
             // clang-format doesn't support formatting a range that starts at the end
             // of the file.
             if (start >= text.Length && text.Length > 0)
                 start = text.Length - 1;
-            string path = GetDocumentParent(view);
-            string filePath = GetDocumentPath(view);
+            string path = VsixUtils.GetDocumentParent(view);
+            string filePath = VsixUtils.GetDocumentPath(view);
 
-            RunClangFormatAndApplyReplacements(text, start, length, path, filePath, view);
+            RunClangFormatAndApplyReplacements(text, start, length, path, filePath, options, view);
         }
 
         /// <summary>
         /// Runs clang-format on the current document
         /// </summary>
-        private void FormatDocument()
+        private void FormatDocument(OptionPageGrid options)
+        {
+            FormatView(VsixUtils.GetCurrentView(), options);
+        }
+
+        private void FormatDocument(Document document, OptionPageGrid options)
+        {
+            FormatView(VsixUtils.GetDocumentView(document), options);
+        }
+
+        private void FormatView(IWpfTextView view, OptionPageGrid options)
         {
-            IWpfTextView view = GetCurrentView();
             if (view == null)
                 // We're not in a text view.
                 return;
 
-            string filePath = GetDocumentPath(view);
+            string filePath = VsixUtils.GetDocumentPath(view);
             var path = Path.GetDirectoryName(filePath);
             string text = view.TextBuffer.CurrentSnapshot.GetText();
 
-            RunClangFormatAndApplyReplacements(text, 0, text.Length, path, filePath, view);
+            RunClangFormatAndApplyReplacements(text, 0, text.Length, path, filePath, options, view);
         }
 
-        private void RunClangFormatAndApplyReplacements(string text, int offset, int length, string path, string filePath, IWpfTextView view)
+        private void RunClangFormatAndApplyReplacements(string text, int offset, int length, string path, string filePath, OptionPageGrid options, IWpfTextView view)
         {
             try
             {
-                string replacements = RunClangFormat(text, offset, length, path, filePath);
+                string replacements = RunClangFormat(text, offset, length, path, filePath, options);
                 ApplyClangFormatReplacements(replacements, view);
             }
             catch (Exception e)
             {
                 var uiShell = (IVsUIShell)GetService(typeof(SVsUIShell));
                 var id = Guid.Empty;
                 int result;
                 uiShell.ShowMessageBox(
                         0, ref id,
                         "Error while running clang-format:",
                         e.Message,
                         string.Empty, 0,
                         OLEMSGBUTTON.OLEMSGBUTTON_OK,
                         OLEMSGDEFBUTTON.OLEMSGDEFBUTTON_FIRST,
                         OLEMSGICON.OLEMSGICON_INFO,
                         0, out result);
             }
         }
 
         /// <summary>
         /// Runs the given text through clang-format and returns the replacements as XML.
         /// 
         /// Formats the text range starting at offset of the given length.
         /// </summary>
-        private string RunClangFormat(string text, int offset, int length, string path, string filePath)
+        private static string RunClangFormat(string text, int offset, int length, string path, string filePath, OptionPageGrid options)
         {
             string vsixPath = Path.GetDirectoryName(
                 typeof(ClangFormatPackage).Assembly.Location);
 
             System.Diagnostics.Process process = new System.Diagnostics.Process();
             process.StartInfo.UseShellExecute = false;
             process.StartInfo.FileName = vsixPath + "\\clang-format.exe";
             // Poor man's escaping - this will not work when quotes are already escaped
             // in the input (but we don't need more).
-            string style = GetStyle().Replace("\"", "\\\"");
-            string fallbackStyle = GetFallbackStyle().Replace("\"", "\\\"");
+            string style = options.Style.Replace("\"", "\\\"");
+            string fallbackStyle = options.FallbackStyle.Replace("\"", "\\\"");
             process.StartInfo.Arguments = " -offset " + offset +
                                           " -length " + length +
                                           " -output-replacements-xml " +
                                           " -style \"" + style + "\"" +
                                           " -fallback-style \"" + fallbackStyle + "\"";
-            if (GetSortIncludes())
+            if (options.SortIncludes)
               process.StartInfo.Arguments += " -sort-includes ";
-            string assumeFilename = GetAssumeFilename();
+            string assumeFilename = options.AssumeFilename;
             if (string.IsNullOrEmpty(assumeFilename))
                 assumeFilename = filePath;
             if (!string.IsNullOrEmpty(assumeFilename))
               process.StartInfo.Arguments += " -assume-filename \"" + assumeFilename + "\"";
             process.StartInfo.CreateNoWindow = true;
             process.StartInfo.RedirectStandardInput = true;
             process.StartInfo.RedirectStandardOutput = true;
             process.StartInfo.RedirectStandardError = true;
public sealed class ClangFormatPackage : Package
                 throw new Exception(process.StandardError.ReadToEnd());
             }
             return output;
         }
 
         /// <summary>
         /// Applies the clang-format replacements (xml) to the current view
         /// </summary>
-        private void ApplyClangFormatReplacements(string replacements, IWpfTextView view)
+        private static void ApplyClangFormatReplacements(string replacements, IWpfTextView view)
         {
             // clang-format returns no replacements if input text is empty
             if (replacements.Length == 0)
                 return;
 
             var root = XElement.Parse(replacements);
             var edit = view.TextBuffer.CreateEdit();
             foreach (XElement replacement in root.Descendants("replacement"))
             {
                 var span = new Span(
                     int.Parse(replacement.Attribute("offset").Value),
                     int.Parse(replacement.Attribute("length").Value));
                 edit.Replace(span, replacement.Value);
             }
             edit.Apply();
         }
-
-        /// <summary>
-        /// Returns the currently active view if it is a IWpfTextView.
-        /// </summary>
-        private IWpfTextView GetCurrentView()
-        {
-            // The SVsTextManager is a service through which we can get the active view.
-            var textManager = (IVsTextManager)Package.GetGlobalService(typeof(SVsTextManager));
-            IVsTextView textView;
-            textManager.GetActiveView(1, null, out textView);
-
-            // Now we have the active view as IVsTextView, but the text interfaces we need
-            // are in the IWpfTextView.
-            var userData = (IVsUserData)textView;
-            if (userData == null)
-                return null;
-            Guid guidWpfViewHost = DefGuidList.guidIWpfTextViewHost;
-            object host;
-            userData.GetData(ref guidWpfViewHost, out host);
-            return ((IWpfTextViewHost)host).TextView;
-        }
-
-        private string GetStyle()
-        {
-            var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
-            return page.Style;
-        }
-
-        private string GetAssumeFilename()
-        {
-            var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
-            return page.AssumeFilename;
-        }
-
-        private string GetFallbackStyle()
-        {
-            var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
-            return page.FallbackStyle;
-        }
-
-        private bool GetSortIncludes()
-        {
-            var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
-            return page.SortIncludes;
-        }
-
-        private string GetDocumentParent(IWpfTextView view)
-        {
-            ITextDocument document;
-            if (view.TextBuffer.Properties.TryGetProperty(typeof(ITextDocument), out document))
-            {
-                return Directory.GetParent(document.FilePath).ToString();
-            }
-            return null;
-        }
-
-        private string GetDocumentPath(IWpfTextView view)
-        {
-            ITextDocument document;
-            if (view.TextBuffer.Properties.TryGetProperty(typeof(ITextDocument), out document))
-            {
-                return document.FilePath;
-            }
-            return null;
-        }
     }
 }

tools/clang-format-vs/ClangFormat/RunningDocTableEventsDispatcher.cs

+using EnvDTE;
+using Microsoft.VisualStudio;
+using Microsoft.VisualStudio.Shell;
+using Microsoft.VisualStudio.Shell.Interop;
+using System.Linq;
+    
+namespace LLVM.ClangFormat
+{
+    // Exposes event sources for IVsRunningDocTableEvents3 events.
+    internal sealed class RunningDocTableEventsDispatcher : IVsRunningDocTableEvents3
+    {
+        private RunningDocumentTable _runningDocumentTable;
+        private DTE _dte;
+
+        public delegate void OnBeforeSaveHander(object sender, Document document);
+        public event OnBeforeSaveHander BeforeSave;
+
+        public RunningDocTableEventsDispatcher(Package package)
+        {
+            _runningDocumentTable = new RunningDocumentTable(package);
+            _runningDocumentTable.Advise(this);
+            _dte = (DTE)Package.GetGlobalService(typeof(DTE));
+        }
+
+        public int OnAfterAttributeChange(uint docCookie, uint grfAttribs)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnAfterAttributeChangeEx(uint docCookie, uint grfAttribs, IVsHierarchy pHierOld, uint itemidOld, string pszMkDocumentOld, IVsHierarchy pHierNew, uint itemidNew, string pszMkDocumentNew)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnAfterDocumentWindowHide(uint docCookie, IVsWindowFrame pFrame)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnAfterFirstDocumentLock(uint docCookie, uint dwRDTLockType, uint dwReadLocksRemaining, uint dwEditLocksRemaining)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnAfterSave(uint docCookie)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnBeforeDocumentWindowShow(uint docCookie, int fFirstShow, IVsWindowFrame pFrame)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnBeforeLastDocumentUnlock(uint docCookie, uint dwRDTLockType, uint dwReadLocksRemaining, uint dwEditLocksRemaining)
+        {
+            return VSConstants.S_OK;
+        }
+
+        public int OnBeforeSave(uint docCookie)
+        {
+            if (BeforeSave != null)
+            {
+                var document = FindDocumentByCookie(docCookie);
+                if (document != null) // Not sure why this happens sometimes
+                {
+                    BeforeSave(this, FindDocumentByCookie(docCookie));
+                }
+            }
+            return VSConstants.S_OK;
+        }
+
+        private Document FindDocumentByCookie(uint docCookie)
+        {
+            var documentInfo = _runningDocumentTable.GetDocumentInfo(docCookie);
+            return _dte.Documents.Cast<Document>().FirstOrDefault(doc => doc.FullName == documentInfo.Moniker);
+        }
+    }
+}

tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs

+using System;
+using System.ComponentModel;
+using System.Text.RegularExpressions;
+
+namespace LLVM.ClangFormat
+{
+    public sealed class TypeConverterUtils
+    {
+        // Generic enum type converter that automatically converts enums <-> strings by
+        // injecting/removing spaces into camel-cased strings. You must derive your own
+        // type and pass the typeof(YourEnum) to the base constructor, rather than provide
+        // a generic type due to limitations of C# attributes.
+        public class EnumStringTypeConverter : EnumConverter
+        {
+            private Type enumType;
+            public EnumStringTypeConverter(Type enumType) : base(enumType)
+            {
+                this.enumType = enumType;
+            }
+
+            public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
+            {
+                if (sourceType == typeof(string))
+                    return true;
+
+                return base.CanConvertFrom(context, sourceType);
+            }
+
+            public override object ConvertFrom(ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value)
+            {
+                string str = value as string;
+
+                if (str != null)
+                {
+                    str.Replace(" ", "");
+                    return Enum.Parse(enumType, str.Replace(" ", ""), false);
+                }
+
+                return base.ConvertFrom(context, culture, value);
+            }
+
+            public override object ConvertTo(ITypeDescriptorContext context, System.Globalization.CultureInfo culture, object value, Type destinationType)
+            {
+                if (destinationType == typeof(string))
+                {
+                    var enumString = ((Enum)value).ToString();
+                    return Regex.Replace(enumString, "([A-Z])", " $1").Trim();
+                }
+
+                return base.ConvertTo(context, culture, value, destinationType);
+            }
+        }
+    }
+}

tools/clang-format-vs/ClangFormat/VsixUtils.cs

+using EnvDTE;
+using Microsoft.VisualStudio.Editor;
+using Microsoft.VisualStudio.Shell;
+using Microsoft.VisualStudio.Shell.Interop;
+using Microsoft.VisualStudio.Text;
+using Microsoft.VisualStudio.Text.Editor;
+using Microsoft.VisualStudio.TextManager.Interop;
+using System;
+using System.IO;
+
+namespace LLVM.ClangFormat
+{
+    internal sealed class VsixUtils
+    {
+        /// <summary>
+        /// Returns the currently active view if it is a IWpfTextView.
+        /// </summary>
+        public static IWpfTextView GetCurrentView()
+        {
+            // The SVsTextManager is a service through which we can get the active view.
+            var textManager = (IVsTextManager)Package.GetGlobalService(typeof(SVsTextManager));
+            IVsTextView textView;
+            textManager.GetActiveView(1, null, out textView);
+
+            // Now we have the active view as IVsTextView, but the text interfaces we need
+            // are in the IWpfTextView.
+            return VsToWpfTextView(textView);
+        }
+
+        public static bool IsDocumentDirty(Document document)
+        {
+            var textView = GetDocumentView(document);
+            var textDocument = GetTextDocument(textView);
+            return textDocument?.IsDirty == true;
+        }
+
+        public static IWpfTextView GetDocumentView(Document document)
+        {
+            var textView = GetVsTextViewFrompPath(document.FullName);
+            return VsToWpfTextView(textView);
+        }
+
+        public static IWpfTextView VsToWpfTextView(IVsTextView textView)
+        {
+            var userData = (IVsUserData)textView;
+            if (userData == null)
+                return null;
+            Guid guidWpfViewHost = DefGuidList.guidIWpfTextViewHost;
+            object host;
+            userData.GetData(ref guidWpfViewHost, out host);
+            return ((IWpfTextViewHost)host).TextView;
+        }
+
+        public static IVsTextView GetVsTextViewFrompPath(string filePath)
+        {
+            // From http://stackoverflow.com/a/2427368/4039972
+            var dte2 = (EnvDTE80.DTE2)Package.GetGlobalService(typeof(SDTE));
+            var sp = (Microsoft.VisualStudio.OLE.Interop.IServiceProvider)dte2;
+            var serviceProvider = new Microsoft.VisualStudio.Shell.ServiceProvider(sp);
+
+            IVsUIHierarchy uiHierarchy;
+            uint itemID;
+            IVsWindowFrame windowFrame;
+            if (VsShellUtilities.IsDocumentOpen(serviceProvider, filePath, Guid.Empty,
+                out uiHierarchy, out itemID, out windowFrame))
+            {
+                // Get the IVsTextView from the windowFrame.
+                return VsShellUtilities.GetTextView(windowFrame);
+            }
+            return null;
+        }
+
+        public static ITextDocument GetTextDocument(IWpfTextView view)
+        {
+            ITextDocument document;
+            if (view != null && view.TextBuffer.Properties.TryGetProperty(typeof(ITextDocument), out document))
+                return document;
+            return null;
+        }
+
+        public static string GetDocumentParent(IWpfTextView view)
+        {
+            ITextDocument document = GetTextDocument(view);
+            if (document != null)
+            {
+                return Directory.GetParent(document.FilePath).ToString();
+            }
+            return null;
+        }
+
+        public static string GetDocumentPath(IWpfTextView view)
+        {
+            return GetTextDocument(view)?.FilePath;
+        }
+    }
+}