[libcamera-devel] [PATCH] utils: checkstyle.py: Fix color bleed

[Tomi Valkeinen]

On 30/05/2022 11:22, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, May 30, 2022 at 10:22:28AM +0300, Tomi Valkeinen wrote:
>> If issue.line is None, the the terminal color is never reset back to
>> normal. This causes the yellow color to bleed.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>>   utils/checkstyle.py | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
>> index 835f2a9f..fa0513f2 100755
>> --- a/utils/checkstyle.py
>> +++ b/utils/checkstyle.py
>> @@ -743,9 +743,9 @@ def check_file(top_level, commit, filename):
>>       if len(issues):
>>           issues = sorted(issues, key=lambda i: i.line_number)
>>           for issue in issues:
>> -            print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue.line_number, issue.msg))
>> +            print('%s#%u: %s%s' % (Colours.fg(Colours.Yellow), issue.line_number, issue.msg, Colours.reset()))
> 
> This could be wrapped.
> 
>>               if issue.line is not None:
>> -                print('+%s%s' % (issue.line.rstrip(), Colours.reset()))
>> +                print('%s+%s%s' % (Colours.fg(Colours.Yellow), issue.line.rstrip(), Colours.reset()))
> 
> I think I would have added
> 
>              sys.stdout.write(Colours.reset())
> 
> here (and dropped it from the previous line) to avoid outputting it
> twice, but that makes no big difference. Either way,

There are many ways to do this, of course. If we want to optimize, we'll 
print Yellow only once, then the text lines, and a reset at the end. But 
that would be a rather pointless optimization.

I thought it's most obvious for the reader and easiest to maintain if we 
print a color and a reset in the same print() call.

In fact, I think it would be better to have a function, colorize(color, 
str), which returns the str with color and reset. This is what e.g. 
xtermcolor module does.

  Tomi