[PATCH] D90109: [clang-tidy] Use ANSI escape codes for --use-color on Windows

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 27 16:36:39 PDT 2020


dblaikie added a comment.

In D90109#2356317 <https://reviews.llvm.org/D90109#2356317>, @aaron.ballman wrote:

> In D90109#2352180 <https://reviews.llvm.org/D90109#2352180>, @dsanders11 wrote:
>
>> Added a few inline comments for clarification.
>>
>> **Note**: I was not able to get a debug build and unit tests working on my Windows set up, so this has only been tested manually. I'm not sure if the unit test added in D79477 <https://reviews.llvm.org/D79477> runs on Windows, but  in theory it should be broken on Windows since it expects ANSI escape codes in the output.
>
> The functionality test specifies `REQUIRES: ansi-escape-sequences` so I'm guessing that test is not run on Windows. The unit test is parsing configuration options, not exercising the options themselves.
>
> What issues did you run into regarding testing, because I feel like the patch should have test coverage (esp given that color vs ANSI escape codes are a bit of an oddity to reason about)?

+1, I'd expect such a test could unconditionally FileCheck the output to test for the escape sequences being present there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90109/new/

https://reviews.llvm.org/D90109



More information about the cfe-commits mailing list