Re: [bitbake-devel] [PATCH v2 3/4] bitbake-setup: print colored diffs

["Antonin Godard" <antonin.godard@bootlin.com>]

On Thu Feb 19, 2026 at 12:13 PM CET, Alexander Kanavin via lists.openembedded.org wrote:
> On Thu, 19 Feb 2026 at 11:18, Antonin Godard <antonin.godard@bootlin.com> wrote:
>> Not sure I follow.
>>
>> def color_enabled() -> bool:
>>     return logger.handlers[0].formatter.color_enabled
>>
>> IMO this is better than the previous version which did:
>>
>> global BBSETUP_COLOR
>> if args.color == 'always' or (args.color == 'auto' and sys.stdout.isatty() and os.environ.get('NO_COLOR', '') == ''):
>>     BBSETUP_COLOR = True
>>
>> That logic is already present in the logger object, in msg.py, when creating the
>> logger:
>>
>> if color == 'always' or (color == 'auto' and output.isatty() and os.environ.get('NO_COLOR', '') == ''):
>>     format.enable_color()
>>
>> The proposal here was to get rid of BBSETUP_COLOR entirely and avoid
>> duplication. Not sure how this is *way* over-engineered, I'd rather see it as a
>> simplification
>
> Apologies, I started writing emails before putting enough coffee in my
> bloodstream and got everyone confused.
>
> What I mean is that run_git_diff() ended up being too complex, because
> it aims to cover all possible use cases of getting a diff. I know I
> had asked for that :) but I wonder if instead of that whole new
> function, we could simply add:
>
> def get_color_param():
>     return "--color={}".format("always" if color_enabled() else "never")
>
> and then tweak all existing invocations of 'diff' and 'git diff', e.g.:
>
> -        bb.process.run('diff -uNr {} {}'.format(file1, file2))
> +        bb.process.run('diff {} -uNr {} {}'.format(get_color_param(),
> file1, file2))
>
> Would be a much simpler change, with the same end result.

Gotcha, just sent a new version :)

Antonin