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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
