[gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Michał Górny]

Update epytest to respect the modern NO_COLOR variable rather than
Portage's old NOCOLOR.  Adjust it to correctly check whether it is set
at all rather than to a specific value, to match the behavior of pytest
itself.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/python-utils-r1.eclass | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
index 394f64a5d139..da9cb820840f 100644
--- a/eclass/python-utils-r1.eclass
+++ b/eclass/python-utils-r1.eclass
@@ -1336,15 +1336,8 @@ epytest() {
 	_python_check_EPYTHON
 	_python_check_occluded_packages
 
-	local color
-	case ${NOCOLOR} in
-		true|yes)
-			color=no
-			;;
-		*)
-			color=yes
-			;;
-	esac
+	local color=yes
+	[[ ${NO_COLOR} ]] && color=no
 
 	local args=(
 		# verbose progress reporting and tracebacks
-- 
2.43.0

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Eli Schwartz]

On 12/2/23 7:44 AM, Michał Górny wrote:
> Update epytest to respect the modern NO_COLOR variable rather than
> Portage's old NOCOLOR.  Adjust it to correctly check whether it is set
> at all rather than to a specific value, to match the behavior of pytest
> itself.
> 
> Signed-off-by: Michał Górny <mgorny@gentoo.org>
> ---
>  eclass/python-utils-r1.eclass | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> index 394f64a5d139..da9cb820840f 100644
> --- a/eclass/python-utils-r1.eclass
> +++ b/eclass/python-utils-r1.eclass
> @@ -1336,15 +1336,8 @@ epytest() {
>  	_python_check_EPYTHON
>  	_python_check_occluded_packages
>  
> -	local color
> -	case ${NOCOLOR} in
> -		true|yes)
> -			color=no
> -			;;
> -		*)
> -			color=yes
> -			;;
> -	esac
> +	local color=yes
> +	[[ ${NO_COLOR} ]] && color=no


[[ -v NO_COLOR ]]

This is processed by the pytest code:

```
    if "NO_COLOR" in os.environ:
        return False
```


>  
>  	local args=(
>  		# verbose progress reporting and tracebacks


-- 
Eli Schwartz

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

On Mon, 2023-12-11 at 13:57 -0500, Eli Schwartz wrote:
> On 12/2/23 7:44 AM, Michał Górny wrote:
> > Update epytest to respect the modern NO_COLOR variable rather than
> > Portage's old NOCOLOR.  Adjust it to correctly check whether it is set
> > at all rather than to a specific value, to match the behavior of pytest
> > itself.
> > 
> > Signed-off-by: Michał Górny <mgorny@gentoo.org>
> > ---
> >  eclass/python-utils-r1.eclass | 11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> > 
> > diff --git a/eclass/python-utils-r1.eclass b/eclass/python-utils-r1.eclass
> > index 394f64a5d139..da9cb820840f 100644
> > --- a/eclass/python-utils-r1.eclass
> > +++ b/eclass/python-utils-r1.eclass
> > @@ -1336,15 +1336,8 @@ epytest() {
> >  	_python_check_EPYTHON
> >  	_python_check_occluded_packages
> >  
> > -	local color
> > -	case ${NOCOLOR} in
> > -		true|yes)
> > -			color=no
> > -			;;
> > -		*)
> > -			color=yes
> > -			;;
> > -	esac
> > +	local color=yes
> > +	[[ ${NO_COLOR} ]] && color=no
> 
> 
> [[ -v NO_COLOR ]]
> 
> This is processed by the pytest code:
> 
> ```
>     if "NO_COLOR" in os.environ:
>         return False
> ```
> 

That looks wrong.  Per [1]:

> […] NO_COLOR environment variable that, when present and not an empty
string (regardless of its value), prevents the addition of ANSI color.

So hey, I'm actually fixing pytest ;-).

[1] https://no-color.org/

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:

>> +	local color=yes
>> +	[[ ${NO_COLOR} ]] && color=no

> [[ -v NO_COLOR ]]

No, this would give the wrong result if NO_COLOR is set to an empty
value. [[ ${NO_COLOR} ]] or [[ -n ${NO_COLOR} ]] is the correct test:

   "Command-line software which adds ANSI color to its output by default
   should check for a NO_COLOR environment variable that, when present
   and not an empty string (regardless of its value), prevents the
   addition of ANSI color." -- https://no-color.org/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Eli Schwartz]

On 12/11/23 2:30 PM, Ulrich Mueller wrote:
>>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:
> 
>>> +	local color=yes
>>> +	[[ ${NO_COLOR} ]] && color=no
> 
>> [[ -v NO_COLOR ]]
> 
> No, this would give the wrong result if NO_COLOR is set to an empty
> value. [[ ${NO_COLOR} ]] or [[ -n ${NO_COLOR} ]] is the correct test:
> 
>    "Command-line software which adds ANSI color to its output by default
>    should check for a NO_COLOR environment variable that, when present
>    and not an empty string (regardless of its value), prevents the
>    addition of ANSI color." -- https://no-color.org/


Again, not according to pytest itself. ;)

Given the commit message says:

"""
Adjust it to correctly check whether it is set at all rather than to a
specific value, to match the behavior of pytest itself.
"""


-- 
Eli Schwartz

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Eli Schwartz]

On 12/11/23 2:27 PM, Michał Górny wrote:
> That looks wrong.  Per [1]:
> 
>> […] NO_COLOR environment variable that, when present and not an empty
> string (regardless of its value), prevents the addition of ANSI color.
> 
> So hey, I'm actually fixing pytest ;-).
> 
> [1] https://no-color.org/


When you say you are fixing pytest, do you mean you are submitting a PR
to pytest to make its behavior align with the (tbh reasonable) behavior
you expected and want?


-- 
Eli Schwartz

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

>>>>> On Mon, 11 Dec 2023, Eli Schwartz wrote:

>> "Command-line software which adds ANSI color to its output by default
>> should check for a NO_COLOR environment variable that, when present
>> and not an empty string (regardless of its value), prevents the
>> addition of ANSI color." -- https://no-color.org/

> Again, not according to pytest itself. ;)

> Given the commit message says:

> """
> Adjust it to correctly check whether it is set at all rather than to a
> specific value, to match the behavior of pytest itself.
> """

The standard is defined by sno-color.org. If pytest does something
different then it is a bug.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] [PATCH] python-utils-r1.eclass: epytest, use NO_COLOR rather than NOCOLOR

[Toralf Förster]

[-- Attachment #1.1: Type: text/plain, Size: 152 bytes --]

On 12/11/23 21:38, Ulrich Mueller wrote:

> The standard is defined by sno-color.org.

http://no-color.org

-- 
Toralf
PGP 23217DA7 9B888F45


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]