[PATCH] D27501: clang-format-vsix: add command to format document

[Antonio Maiorano via Phabricator via cfe-commits]

amaiorano added inline comments.


================
Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76
+        <Strings>
+          <ButtonText>Clang Format Document</ButtonText>
+        </Strings>
----------------
klimek wrote:
> amaiorano wrote:
> > klimek wrote:
> > > hans wrote:
> > > > amaiorano wrote:
> > > > > hans wrote:
> > > > > > I think File would be better than Document when referring to source code.
> > > > > > 
> > > > > > But it seems a little annoying to need two menu alternatives. Could we make the regular "Clang Format" option just format the whole file if there is currently no selection, or would that be confusing?
> > > > > The reason I chose "Document" is that it mimics the existing functionality in Visual Studio:
> > > > > {F2661117}
> > > > > 
> > > > > As you can see, "Ctrl+K, Ctrl+F" is bound to format selection by default; which I assumed is the reason "Ctrl+R, Ctrl+F" was used for Clang Format (Selection). So along the same lines, I bound "Ctrl + R, Ctrl + D" to Clang Format Document.
> > > > > 
> > > > > I don't really have a problem with having multiple menu options, although we could have both of them underneath a "Clang Format" top menu, with "Format Selection" and "Format Document" as sub menu items.
> > > > > 
> > > > > As for annoyance in menus, I think most developers would reach for keyboard shortcuts in this case anyway. Furthermore, the next feature I want to add, building on top of this one, is allowing the user to enable "Format on Save", which would make this even easier to use.
> > > > I see, I didn't realize they call it documents. And if they also have separate commands for formating selection and formating the whole file, maybe that makes sense for us too.
> > > > 
> > > > Right, I also imagine folks would use this from the keyboard, but there too it's annoying to have two shortcuts for the same thing. But again, if that's how it's generally done in VS...
> > > > 
> > > > I'd like to hear from Manuel on this patch too, though.
> > > Clang-format currently formats the current line / statement if there is no selection, and that's how most people I know use it.
> > > Formatting the whole file by default is not what clang-format is optimized for, and highly disruptive (eclipse does that, and it's annoying).
> > > I'm more wondering why we need a setting for this at all - isn't ctrl-a,ctrl-k,ctrl-f equivalent to formatting the whole file?
> > @klimek My own experience, which also seems to match those of others online, is that clang-format is a fantastic tool for making sure your code-base follows a strict formatting standard. What many people do is install Git pre-stage hooks, for instance, to run clang-format across entire source files, to ensure they're formatted properly. So even if it's not optimized for that, it works very well :)
> > 
> > Using clang-format to format only the current line doesn't guarantee that everything will be formatted accordingly. My own experience is that it sometimes formats the line differently than if you format the entire document. Or perhaps you end up writing a few lines of code, and formatting current line doesn't pick up all the lines you've modified. As for ctrl-a,ctrl-r-ctrl-f, the problem with it is that you lose your cursor position, as ctrl-a will place it at the end of the file.
> > 
> > You also mention that formatting the entire file is "highly disruptive" like in eclipse. I'm not sure how eclipse handles that, but so far VS seems to do a pretty good job. It manages to maintain breakpoint positions quite well, it manages to keep the cursor in the same place, and it manages to keep the relative location of the code where the cursor is at visible on screen. I suspect part of the reason it works well in VS is because Microsoft made sure it worked for their own format document feature (ctrl-k,ctrl-d).
> > 
> > As I mentioned earlier, this is a stepping stone towards adding a feature to "format on save", for which I'd like to offer the ability to choose "current line/selection" and "document". [[ https://github.com/Elders/VSE-FormatDocumentOnSave | Here's an example ]] of an extension that offers this feature, but for VS's built-in formatting.
> Note: with "highly disruptive" I meant only if the default behavior without any selection is to format the whole file, as that basically kills the workflow; that was what Hans suggested.
> 
> I'm not opposed to having an extra key binding if enough folks want it and there is enough benefit - the argument to not move your cursor is a good point.
Thanks! Now I just need to get my svn submit working. Chris Latner created my account, etc. but when I tried to submit another accepted change, it didn't work. Anyway, will figure it out. Thanks again :)


https://reviews.llvm.org/D27501