| Message ID | 20240215112719.392617-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | sort-makefile-lines.py: Allow '_' in name and "^# name" | expand |
* 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. Thanks, Florian
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]) if var: # Remember the index and the name. startmarks.append((i, var.group(1))) > Thanks, > Florian >
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 >> > >
On Feb 15 2024, H.J. Lu wrote: > @@ -139,8 +139,8 @@ def sort_makefile_lines(): > # must have the matching comment name for it to be valid. > rangemarks = [] > for sm in startmarks: > - # Look for things like " # var" to end the sorted list. > - reg = r'^ # ' + sm[1] + r'$' > + # Look for things like "^ *# var" to end the sorted list. > + reg = r'^ *# ' + sm[1] + r'$' The comment is a bit confusing now, as the actual regexp is "^ *# var$". I think the old comment still fits.
On Thu, Feb 15, 2024 at 4:24 AM Carlos O'Donell <carlos@redhat.com> wrote: > > 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! Done. Thanks. > > if var: > > # Remember the index and the name. > > startmarks.append((i, var.group(1))) > > > >> Thanks, > >> Florian > >> > > > > > > -- > Cheers, > Carlos. >
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))) @@ -139,8 +139,8 @@ def sort_makefile_lines(): # must have the matching comment name for it to be valid. rangemarks = [] for sm in startmarks: - # Look for things like " # var" to end the sorted list. - reg = r'^ # ' + sm[1] + r'$' + # Look for things like "^ *# var" to end the sorted list. + reg = r'^ *# ' + sm[1] + r'$' for j in range(sm[0] + 1, len(lines)): if re.search(reg, lines[j]): # Remember the block to sort (inclusive).