[PATCH] D29221: clang-format-vsix: "format on save" feature

[Antonio Maiorano via Phabricator via cfe-commits]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29221.86046.patch
Type: text/x-patch
Size: 25944 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170127/644b7328/attachment-0001.bin>