Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[Carlos O'Donell <carlos@redhat.com>]

On 2/15/24 07:19, H.J. Lu wrote:
> On Thu, Feb 15, 2024 at 4:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>>> '_' is used in Makefile variable names and many variables end with
>>> "^# name".  Relax sort-makefile-lines.py to allow '_' in name and
>>> "^# name" as variable end.  This fixes BZ #31385.
>>> ---
>>>  scripts/sort-makefile-lines.py | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
>>> index f65ee40e27..ea02412d67 100755
>>> --- a/scripts/sort-makefile-lines.py
>>> +++ b/scripts/sort-makefile-lines.py
>>> @@ -129,7 +129,7 @@ def sort_makefile_lines():
>>>      for i in range(len(lines)):
>>>          # Look for things like "var = \", "var := \" or "var += \"
>>>          # to start the sorted list.
>>> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
>>> +        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
>>>          if var:
>>>              # Remember the index and the name.
>>>              startmarks.append((i, var.group(1)))
>>
>> Please keep the literal - at the end of the bracket expression.  I think
>> it's easier to read even if it may be semantically the same.
> 
> Did you mean like this?
> 
> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> index f65ee40e27..d791789671 100755
> --- a/scripts/sort-makefile-lines.py
> +++ b/scripts/sort-makefile-lines.py
> @@ -129,7 +129,7 @@ def sort_makefile_lines():
>      for i in range(len(lines)):
>          # Look for things like "var = \", "var := \" or "var += \"
>          # to start the sorted list.
> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> +        var = re.search(r'^([-_a-zA-Z0-9]*) [\+:]?\= \\$', lines[i])

Yes, exactly, I agree with Florian here you want to show that the literal '-' and '_'
are not part of a range in the character class. So either start or end is fine with
me.

Looking forward to v2. Thanks for fixing this!

>          if var:
>              # Remember the index and the name.
>              startmarks.append((i, var.group(1)))
> 
>> Thanks,
>> Florian
>>
> 
> 

-- 
Cheers,
Carlos.