[llvm-commits] [llvm] r72854 - in /llvm/trunk: include/llvm/Support/raw_ostream.h include/llvm/System/Process.h lib/Support/raw_ostream.cpp lib/System/Unix/Process.inc lib/System/Win32/Process.inc

[Török Edwin]

On 2009-06-04 10:49, Duncan Sands wrote:
> Hi Edwin,
>
>   
>> +      /// This function returns the colorcode escape sequences, and sets Len to
>> +      /// the length of the escape sequence.
>>     
>
> what is Len?  It's not a parameter of OutputColor at least.
>   

It was removed, I updated the comment.

>   
>> +      static const char *OutputColor(char c, bool bold, bool bg);
>> +
>> +      /// Same as OutputColor, but only enables the bold attribute.
>> +      static const char *OutputBold(bool bg);
>>     
>
> I see that OutputColor takes the character 'c' but OutputBold doesn't.
> Is that future proof?
>   

OutputBold only changes the bold attribute and keeps the color intact, I
don't see what we could use
'c' for.
On Unix it simply outputs the escape sequence to turn bold on,
on Win32 queries the current attributes, turns intensity on, and changes
attributes.

>   
>> +// it always has colors
>>     
>
> it -> It
> Missing fullstop at end of comment.
>
>   
>> +bool Process::StandardOutHasColors() {
>> +  return StandardOutIsDisplayed();
>> +}
>> +namespace {
>>     
>
> Missing blank line before namespace.
>
>   

Thanks Duncan!

--Edwin