Re: [PATCH iptables] iptables-test.py: print with color escapes only when stdout isatty

["Štěpán Němec" <snemec@redhat.com>]

On Fri, 03 Sep 2021 14:52:50 +0200
Phil Sutter wrote:

> With one minor nit:
>
>> diff --git a/iptables-test.py b/iptables-test.py
>> index 90e07feed365..e8fc0c75a43e 100755
>> --- a/iptables-test.py
>> +++ b/iptables-test.py
>> @@ -32,22 +32,25 @@ EXTENSIONS_PATH = "extensions"
>>  LOGFILE="/tmp/iptables-test.log"
>>  log_file = None
>>  
>> +STDOUT_IS_TTY = sys.stdout.isatty()
>>  
>> -class Colors:
>> -    HEADER = '\033[95m'
>> -    BLUE = '\033[94m'
>> -    GREEN = '\033[92m'
>> -    YELLOW = '\033[93m'
>> -    RED = '\033[91m'
>> -    ENDC = '\033[0m'
>> +def maybe_colored(color, text):
>> +    terminal_sequences = {
>> +        'green': '\033[92m',
>> +        'red': '\033[91m',
>> +    }
>> +
>> +    return (
>> +        terminal_sequences[color] + text + '\033[0m' if STDOUT_IS_TTY else text
>> +    )
>
> I would "simplify" this into:
>
> | if not sys.stdout.isatty():
> | 	return text
> | return terminal_sequences[color] + text + '\033[0m'

...which could be further simplified by dropping 'not' and swapping the
two branches.

So there seem to be two things here: double return instead of
conditional expression, and calling the isatty method for every relevant
log line instead of once at the beginning.

I deliberately avoided the latter and I think I still prefer the
conditional expression to multiple return statements, too, but either
way should keep the escape sequences out of the log files and I don't
feel strongly about it.

Thanks,

  Štěpán