[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] 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] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2017-01-03 Thread Cameron via Phabricator via cfe-commits
cameron314 added a comment. >> 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? > > No one's bothered looking into it yet. Which tests? We run the

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

2016-12-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment. In https://reviews.llvm.org/D27440#626477, @amaiorano wrote: > 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

[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 + erro

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

2016-12-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment. `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 + error_message in the returned error, there is also `llvm::StringError`. An

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

2016-12-19 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a reviewer: ioeric. klimek added a comment. +eric, who has some experience llvm::Error'ing our interfaces :) llvm::ErrorOr seems like the right approach here? https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@list

[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 .clang-f

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

2016-12-16 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Yes.. return non-zero seems right. This is an error condition. https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 dire

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

2016-12-16 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. 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 directories (not sure whether we currently do that). https://reviews.

[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] 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] D27440: clang-format-vsix: fail when clang-format outputs to stderr

2016-12-06 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a reviewer: djasper. klimek added a comment. Adding djasper, who had brought forward the idea. https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[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? W

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

2016-12-06 Thread Manuel Klimek via Phabricator via cfe-commits
klimek added a comment. 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? https://reviews.llvm.org/D27440 __

[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 stat

[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