[llvm-commits] [PATCH] ANSI colors for clang!

[Török Edwin]

On 2009-06-04 10:00, Chris Lattner wrote:
> On Jun 1, 2009, at 11:20 AM, Török Edwin wrote:
>>>> A lot of the complexity of the patch looks like it has to do with
>>>> Windows support.  Please find someone with a real VC++ build to verify
>>>> that it actually works :)
>>>>
>>
>> I've tested it myself on VC++2k8 Express, by generating a .sln using
>> cmake-gui
>> (Express edition didn't like the .sln in win32/, but worked fine with
>> the one from cmake).
>> Apart from a compilation failure in ilist.h (solved by the attached
>> patch) everything went smoothly,
>> and my (blindly) written color support for win32 consoles actually
>> worked the first time I tried:
>> it printed an error that it can't find string.h in color!
>
> Ok, other questions:
>
> +raw_ostream &raw_fd_ostream::resetColor() {
> +  if (sys::Process::ColorNeedsFlush())
> +    flush();
> +  const char *colorcode = sys::Process::ResetColor();
> +  if (colorcode) {
> +    unsigned len = strlen(colorcode);
> +    write(colorcode, len);
> +    // don't account colors towards output characters
> +    pos -= len;
> +  }
> +  return *this;
>
> Is write followed by subtracting from "pos" really safe here?  What if
> write doesn't do a flush?  

If it does a flush it is as if the write didn't happen at all, hence safe.
> Does it matter?

pos is not exposed publicly, its only exposed via tell(), which is pos +
numbytesinbuffer.
Even if pos wraps because there was no flush, tell() will still return
the correct result, and pos isn't used anywhere else.

>
> +const char *Process::OutputColor(char code, bool bold, bool bg) {
> +  const char *ret = colorcodes[bg?1:0][bold?1:0][code&7];
> +  return ret;
>
> How about "return colorcodes[..."

Yeah, I missed that while deleting a parameter ;)

>
>
> Otherwise, looks great!  Please apply after addressing the two things
> above (if needed).  Thanks Edwin,

Thanks, applied.

Best regards,
--Edwin