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

2017-04-05 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL299543: clang-format-vsix: Add "Format on Save" feature (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D29221?vs=89815=94225#toc Repository: rL LLVM

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

2017-03-30 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D29221#713902, @hans wrote: > In https://reviews.llvm.org/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

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

2017-03-11 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/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

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

2017-02-27 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D29221#687438, @amaiorano wrote: > In https://reviews.llvm.org/D29221#687425, @hans wrote: > > > In https://reviews.llvm.org/D29221#686865, @amaiorano wrote: > > > > > Made changes based on @hans 's feedback. > > > > > > I looked again at

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

2017-02-27 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D29221#687425, @hans wrote: > In https://reviews.llvm.org/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

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

2017-02-26 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 89815. amaiorano added a comment. 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

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

2017-02-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:276 + +if (Vsix.IsDocumentDirty(document)) +{ hans wrote: > Perhaps return early if `!Vsix.IsDocumentDirty(document)` instead? Yes, will

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

2017-02-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:39 private string style = "file"; +private bool formatOnSaveEnabled = false; +private string formatOnSaveFileExtensions = hans wrote: >

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

2017-02-17 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Hello again. Just wondering if anyone could take a look at this? I updated the patch a week ago :) Thanks! https://reviews.llvm.org/D29221 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2017-02-10 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 88000. amaiorano added a comment. - 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

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

2017-02-07 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/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

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

2017-02-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. 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 :)

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

2017-01-27 Thread 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.

[PATCH] D28983: clang-format: remove tests that assume no config file will be found as this is not always the case

2017-01-23 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292787: clang-format: remove tests that assume no config file will be found as this is… (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D28983?vs=85247=85366#toc Repository:

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2017-01-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano abandoned this revision. amaiorano added a comment. This change is no longer needed since clang-format now returns failure status (non-zero) whenever it writes to stderr (e.g. on parse error) since https://llvm.org/svn/llvm-project/cfe/trunk@292174 (https://reviews.llvm.org/D28081)

[PATCH] D28983: clang-format: remove tests that assume no config file will be found as this is not always the case

2017-01-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. Herald added a subscriber: klimek. These tests fail for developers who place their build directories under the llvm root directory because llvm's own .clang-format file will be found. Anyway these cases are covered by FormatStyle.GetStyleOfFile tests

[PATCH] D28943: [clang-format] Remove redundant test in style-on-command-line.cpp

2017-01-20 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28943#651489, @ioeric wrote: > In https://reviews.llvm.org/D28943#651488, @amaiorano wrote: > > > In https://reviews.llvm.org/D28943#651470, @ioeric wrote: > > > > > @amaiorano: The test itself is correct. It's just that this test failed >

[PATCH] D28943: [clang-format] Remove redundant test in style-on-command-line.cpp

2017-01-20 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28943#651470, @ioeric wrote: > @amaiorano: The test itself is correct. It's just that this test failed in > our internal test. We could've fixed it internally, but the fix would be > ugly. Since the intended behavior is already covered in

[PATCH] D28844: clang-format: fix fallback style set to "none" not formatting

2017-01-19 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292562: clang-format: fix fallback style set to "none" not always formatting (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D28844?vs=84795=85074#toc Repository: rL LLVM

[PATCH] D28844: clang-format: fix fallback style set to "none" not formatting

2017-01-17 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: lib/Format/Format.cpp:1906 - // FIXME: If FallbackStyle is explicitly "none", format is disabled. - if (!getPredefinedStyle(FallbackStyle, Style.Language, )) -return make_string_error("Invalid fallback style \"" +

[PATCH] D28844: clang-format: fix fallback style set to "none" not formatting

2017-01-17 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. This change fixes the fact that fallback style set to "none" should not format. Without this change, fallback style "none" ends up applying LLVM formatting. https://reviews.llvm.org/D28844 Files: lib/Format/Format.cpp test/Format/style-on-command-line.cpp

[PATCH] D28315: Update tools to use new getStyle API

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292175: Update tools to use new getStyle API (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D28315?vs=84539=84611#toc Repository: rL LLVM https://reviews.llvm.org/D28315

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL292174: clang-format: Make GetStyle return Expected instead of FormatStyle (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D28081?vs=84538=84610#toc Repository: rL LLVM

[PATCH] D28315: Update tools to use new getStyle API

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#647212, @ioeric wrote: > In https://reviews.llvm.org/D28315#647154, @amaiorano wrote: > > > In https://reviews.llvm.org/D28315#647103, @ioeric wrote: > > > > > Let me know when broken tests are fixed and this patch (and the > > >

[PATCH] D28315: Update tools to use new getStyle API

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#647103, @ioeric wrote: > Let me know when broken tests are fixed and this patch (and the corresponding > patch) is ready again for review. Also let me know if you need any help. I updated the two patches after rebasing on latest

[PATCH] D28315: Update tools to use new getStyle API

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 84539. amaiorano added a comment. Rebased changes on latest. No functional changes in this diff since last. All tests pass (on Windows). https://reviews.llvm.org/D28315 Files: change-namespace/ChangeNamespace.cpp

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-16 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 84538. amaiorano added a comment. Rebased changes on latest, no functional change in this diff. https://reviews.llvm.org/D28081 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Tooling/Refactoring.cpp

[PATCH] D28315: Update tools to use new getStyle API

2017-01-15 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: change-namespace/ChangeNamespace.cpp:892 + llvm::errs() << llvm::toString(Style.takeError()) << "\n"; + continue; +} amaiorano wrote: > amaiorano wrote: > > ioeric wrote: > > > I'd still like to apply

[PATCH] D28315: Update tools to use new getStyle API

2017-01-11 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: change-namespace/ChangeNamespace.cpp:892 + llvm::errs() << llvm::toString(Style.takeError()) << "\n"; + continue; +} amaiorano wrote: > ioeric wrote: > > I'd still like to apply replacements that are not

[PATCH] D28315: Update tools to use new getStyle API

2017-01-11 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 83952. amaiorano added a comment. Fixed code in ClangApplyReplacementsMain.cpp so that we only handle the

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-11 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 83949. amaiorano added a comment. Fixed Format/style-on-command-line.cpp test to match new expected output. https://reviews.llvm.org/D28081 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Tooling/Refactoring.cpp

[PATCH] D28419: clang-tools-extra: add gitattributes to disable EOL conversions on checkout of test source files

2017-01-10 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Last night, I realized that this problem extends to the clang/tests as well, not just those in clang-tools-extra. Is there someone who knows Windows, Git, and the testing pipeline that can weigh in more on this topic? https://reviews.llvm.org/D28419

[PATCH] D28315: Update tools to use new getStyle API

2017-01-09 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#641032, @amaiorano wrote: > In https://reviews.llvm.org/D28315#639559, @ioeric wrote: > > > I ran `ninja check-all` with https://reviews.llvm.org/D28081 and > > https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed,

[PATCH] D28315: Update tools to use new getStyle API

2017-01-09 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#639559, @ioeric wrote: > I ran `ninja check-all` with https://reviews.llvm.org/D28081 and > https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed, namely: > > Clang :: Format/style-on-command-line.cpp This was the

[PATCH] D28315: Update tools to use new getStyle API

2017-01-09 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#639559, @ioeric wrote: > I ran `ninja check-all` with https://reviews.llvm.org/D28081 and > https://reviews.llvm.org/D28315 (in Linux), and some lit tests failed, namely: > > Clang :: Format/style-on-command-line.cpp > Clang

[PATCH] D28419: clang-tools-extra: add gitattributes to disable EOL conversions on checkout of test source files

2017-01-09 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Hello, sorry for the lack of details here. I will re-run the tests later and report here the tests that are failing along with output. The main issue is that when using Git on Windows, by default the installer will set core.autocrlf to true, which is a Git config

[PATCH] D28419: clang-tools-extra: add gitattributes to disable EOL conversions on checkout of test source files

2017-01-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: malcolm.parsons, ioeric. amaiorano added a subscriber: cfe-commits. Certain tests expect their input files to have LF line endings, while others expect CRLF. On Windows, when using Git with core.autocrlf=true, these tests fail because

[PATCH] D28315: Update tools to use new getStyle API

2017-01-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. @ioeric Okay, check-clang-tools passes with my changes. FYI, tests were originally failing for me (without my changes) because I use Git, and it's configured to convert line endings to Windows (crlf) on checkout, but some of the tools tests specify extract offsets

[PATCH] D28315: Update tools to use new getStyle API

2017-01-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#636888, @hokein wrote: > In https://reviews.llvm.org/D28315#636821, @amaiorano wrote: > > > Is there a way to run tests without ninja? I'm on Windows. If not, I'll use > > my Linux VM. > > > It is not related to the build system.

[PATCH] D28315: Update tools to use new getStyle API

2017-01-05 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28315#636670, @hokein wrote: > In https://reviews.llvm.org/D28315#635865, @amaiorano wrote: > > > I made these changes, and they build, but I didn't really test them. Are > > there unit tests for these tools that I can run? > > > If you

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-04 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. @ioeric I think you removed cfe-commits as a reviewer, then added it back, but it didn't work. Should I re-add? https://reviews.llvm.org/D28081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28315: Update tools to use new getStyle API

2017-01-04 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. I made these changes, and they build, but I didn't really test them. Are there unit tests for these tools that I can run? https://reviews.llvm.org/D28315 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28315: Update tools to use new getStyle API

2017-01-04 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: ioeric, cfe-commits, klimek, djasper. See https://reviews.llvm.org/D28081 for the changes to getStyle This change will be committed right after https://reviews.llvm.org/D28081. https://reviews.llvm.org/D28315 Files:

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-03 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28081#633786, @ioeric wrote: > The patch LGTM now. I'll accept both this and the one for clang-tool-extra > when it is ready. > > Regarding builbots, we have bots that continually run builds/tests > (http://lab.llvm.org:8011/). Many

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-03 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 82896. amaiorano added a comment. Minor comment change, turned the ObjC test into a non-fixture test, and renamed FormatStyleOrError to FormatStyle in format function. https://reviews.llvm.org/D28081 Files: include/clang/Format/Format.h

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2017-01-03 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27440#634043, @cameron314 wrote: > >> Thanks, I'll check these out! Btw, I noticed that the clang-format tests > >> are non-Windows due to path assumptions. Is this a lost cause, or just > >> something no one's bothered to look into yet?

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-02 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 82831. amaiorano added a comment. More changes as suggested by @ioeric. I asked a question earlier about clang-tools-extras and understanding how not to break the build bots. If you can shed some light on that, I'd appreciate it, thanks :)

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-02 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: unittests/Format/FormatTestObjC.cpp:72 TEST_F(FormatTestObjC, DetectsObjCInHeaders) { - Style = getStyle("LLVM", "a.h", "none", "@interface\n" + Style = *getStyle("LLVM", "a.h", "none", "@interface\n"

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-02 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D28081#633521, @ioeric wrote: > Some nits. Some is almost good :) > > BTW, do you have clang-tools-extra in your source tree? There are also some > references in the subtree to the changed interface. It would be nice if you > could also

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-02 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 82814. amaiorano added a comment. Reverted the FallbackStyle code and added a FIXME as @ioeric suggested. I'll fix the fallback style "none" bug in a separate change. https://reviews.llvm.org/D28081 Files: include/clang/Format/Format.h

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2017-01-01 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: lib/Format/Format.cpp:1900 - if (!getPredefinedStyle(FallbackStyle, Style.Language, )) { -llvm::errs() << "Invalid fallback style \"" << FallbackStyle - << "\" using LLVM style\n"; -return Style; - } +

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-31 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Hello everyone, so after a few more tests, I've uncovered a bug and perhaps a different meaning for fallback style. First, the bug: if you set fallback style to "none", clang-format will perform no replacements. This happens because getStyle will first initialize its

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-29 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 82708. amaiorano added a comment. Updated with the changes @ioeric suggested. https://reviews.llvm.org/D28081 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Tooling/Refactoring.cpp tools/clang-format/ClangFormat.cpp

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-28 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: lib/Format/Format.cpp:1984 +// If so, can't return this error here... +return make_string_error("Configuration file(s) do(es) not support " + + getLanguageName(Language) + ": " +

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-27 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Thank you, @ioeric for your feedback! Comment at: include/clang/Format/Format.h:862 /// /// \returns FormatStyle as specified by ``StyleName``. If no style could be /// determined, the default is LLVM Style (see ``getLLVMStyle()``).

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-24 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. One more thing I forgot to mention is that this change comes from a discussion we had on this other change: https://reviews.llvm.org/D27440 where @djasper agreed that "fallback-style should only be used when there is no .clang-format file. If we find one, and it

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-23 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: unittests/Format/FormatTestObjC.cpp:72 TEST_F(FormatTestObjC, DetectsObjCInHeaders) { - Style = getStyle("LLVM", "a.h", "none", "@interface\n" + Style = *getStyle("LLVM", "a.h", "none", "@interface\n"

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

2016-12-21 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: lib/Format/Format.cpp:1920-1922 + std::error_code EC = FS->makeAbsolute(Path); + assert(!EC); + (void)EC; klimek wrote: > I think if makeAbsolute doesn't work, we will probably want to err out here: > if (EC) { >

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

2016-12-20 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 82185. amaiorano added a comment. As agreed, modified getStyle to make use of vfs::FileSystem::makeAbsolute. I would modify the body of the commit as follows: Modify getStyle to use vfs::FileSystem::makeAbsolute just like FS.addFile does, rather than

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

2016-12-20 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27971#627543, @klimek wrote: > Why isn't the right solution to make getStyle() use vfs::FileSystem? > Generally, everything in clang-format (well, in clang) should use > vfs::FileSystem for file access. You're absolutely right, this is

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

2016-12-19 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: klimek, djasper, hans, cfe-commits. This is more a workaround than a real fix. The real problem is that FS.addFile uses clang::vfs::FileSystem::makeAbsolute to convert the input path to an absolute path, while getStyle uses

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-19 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27440#626415, @ioeric wrote: > `llvm::ErrorOr` carries `std::error_code`. If you want richer information > (e.g. error_code + error message), `llvm::Expcted` and `llvm::Error` are > your friends. > > FYI, if you only need error_code +

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-18 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27440#624917, @djasper wrote: > Yes.. return non-zero seems right. This is an error condition. Hi @djasper , I started looking into making the changes to clang-format to have it return an error code when it's unable to parse the

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-16 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27440#624773, @djasper wrote: > I agree that fallback-style should only be used when there is no > .clang-format file. If we find one, and it doesn't parse correctly, we should > neither use the fallback style nor scan in higher-level

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-15 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. Hi @djasper, just curious about your opinion on this change, and on the conversation following it, specifically regarding whether clang-format should really use the fallback style when failing to parse a .clang-format file. Thanks! https://reviews.llvm.org/D27440

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

2016-12-15 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289910: clang-format-vsix: add command to format document (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D27501?vs=80533=81701#toc Repository: rL LLVM

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

2016-12-15 Thread Antonio Maiorano via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL289909: clang-format-vsix: add a date stamp to the VSIX version number to ensure… (authored by amaiorano). Changed prior to commit: https://reviews.llvm.org/D27438?vs=80527=81696#toc Repository: rL

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

2016-12-12 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + + Clang Format Document + klimek wrote: > amaiorano wrote: > > klimek wrote: > > > hans wrote: > > > > amaiorano wrote: > > > > > hans

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

2016-12-09 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. (NOTE, I forgot to click Submit on the reply to @klimek a couple days ago!) Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + + Clang Format Document + klimek wrote: > hans wrote: > >

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

2016-12-07 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + + Clang Format Document + hans wrote: > I think File would be better than Document when referring to source code. > > But it seems a

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

2016-12-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 80533. amaiorano added a comment. My first patch accidentally included the changes from https://reviews.llvm.org/D27440 https://reviews.llvm.org/D27501 Files: tools/clang-format-vs/.gitignore tools/clang-format-vs/ClangFormat/ClangFormat.vsct

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

2016-12-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: hans, zturner, klimek. amaiorano added a subscriber: cfe-commits. Bound to Ctrl+R, Ctrl+D by default. Also added section on how to debug the extension to the Readme. https://reviews.llvm.org/D27501 Files:

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

2016-12-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. > Yes, I think this is still a worthwhile change. Let's add the hour and minute > and get it landed. Alright, added hour and minute. How do we land this change? Is this something I can now do myself? Or would you need to commit this for me again? I have a couple of

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

2016-12-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano updated this revision to Diff 80527. amaiorano added a comment. Added hour and minute. https://reviews.llvm.org/D27438 Files: tools/clang-format-vs/CMakeLists.txt Index: tools/clang-format-vs/CMakeLists.txt === ---

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-06 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27440#614337, @klimek wrote: > Pondering this a bit - one question is whether we should make clang-format > not return 0 if we pass -fallback-style=none (it already has this option) and > we can't figure out a style. What do you think?

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

2016-12-05 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. In https://reviews.llvm.org/D27438#614219, @hans wrote: > When building the snapshots and releases, I usually put the SVN revision > number in there to avoid this same problem: > http://llvm-cs.pcc.me.uk/utils/release/build_llvm_package.bat#25 Ah, right, I was

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-05 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano added a comment. With this change, I now get the following message box when my .clang-format has an invalid field named "SomeInvalidField": F2652560: 0.png I do have to wonder whether the real bug is the fact that clang-format returns a success

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-05 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: klimek, hans, zturner. amaiorano added a subscriber: cfe-commits. When clang-format outputs to stderr but returns 0, the extension will format the code anyway. This happens, for instance, when there's a syntax error or unknown value in

[PATCH] D27438: clang-format-vsix: add a date stamp to the VSIX version number to ensure upgradability

2016-12-05 Thread Antonio Maiorano via Phabricator via cfe-commits
amaiorano created this revision. amaiorano added reviewers: klimek, hans, zturner. amaiorano added a subscriber: cfe-commits. Herald added a subscriber: mgorny. Presently, the version number of the VSIX matches the LLVM version number. However, as this number doesn't change often, it means that