amaiorano created this revision. 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 <http://stackoverflow.com/a/31066160/4039972>; furthermore, there is a VS extension that provides this functionality <https://marketplace.visualstudio.com/items?itemName=mynkow.FormatdocumentonSave> 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: F3029838: pasted_file <https://reviews.llvm.org/F3029838> 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! https://reviews.llvm.org/D29221 Files: tools/clang-format-vs/ClangFormat/ClangFormat.csproj tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs tools/clang-format-vs/ClangFormat/RunningDocTableEventsDispatcher.cs tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs tools/clang-format-vs/ClangFormat/VsixUtils.cs
Index: tools/clang-format-vs/ClangFormat/VsixUtils.cs =================================================================== --- /dev/null +++ tools/clang-format-vs/ClangFormat/VsixUtils.cs @@ -0,0 +1,96 @@ +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; + } + } +} Index: tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs =================================================================== --- /dev/null +++ tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs @@ -0,0 +1,54 @@ +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); + } + } + } +} Index: tools/clang-format-vs/ClangFormat/RunningDocTableEventsDispatcher.cs =================================================================== --- /dev/null +++ tools/clang-format-vs/ClangFormat/RunningDocTableEventsDispatcher.cs @@ -0,0 +1,79 @@ +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); + } + } +} Index: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs =================================================================== --- tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs +++ tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs @@ -12,30 +12,48 @@ // //===----------------------------------------------------------------------===// -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 { @@ -74,7 +92,14 @@ } } - [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" + @@ -121,7 +146,7 @@ } } - [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) " + @@ -142,7 +167,7 @@ } } - [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" + @@ -154,29 +179,69 @@ 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) { @@ -195,6 +260,11 @@ } #endregion + OptionPageGrid GetUserOptions() + { + return (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid)); + } + private void MenuItemCallback(object sender, EventArgs args) { var mc = sender as System.ComponentModel.Design.MenuCommand; @@ -204,21 +274,61 @@ 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; @@ -231,34 +341,43 @@ // 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) @@ -283,7 +402,7 @@ /// /// 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); @@ -293,16 +412,16 @@ 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)) @@ -352,7 +471,7 @@ /// <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) @@ -369,70 +488,5 @@ } 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; - } } } Index: tools/clang-format-vs/ClangFormat/ClangFormat.csproj =================================================================== --- tools/clang-format-vs/ClangFormat/ClangFormat.csproj +++ tools/clang-format-vs/ClangFormat/ClangFormat.csproj @@ -214,6 +214,9 @@ </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">
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits