[Yum-devel] [PATCH 1/3] Simplify some regexps.

James Antill james at fedoraproject.org
Wed Oct 28 18:12:57 UTC 2009


On Wed, 2009-10-28 at 19:06 +0200, Ville Skyttä wrote:
> ---
>  yum/misc.py    |    8 ++++----
>  yum/yumRepo.py |    4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)

 Does removing the ^ from the regexp make any difference? Coming from a
perl/grep background I always expect that they are needed, so removing
them is likely to confuse ... esp. as sub/search don't have the retarded
behaviour of match.
 But I'm not going to NAK it for that, if you like it.

 Everything seems fine, so ACK, some minor comments...

[...]
>      # Normalise newlines
> -    rawkey = re.compile('(\n|\r\n|\r)').sub('\n', rawkey)
> +    rawkey = re.sub('\r\n?', '\n', rawkey)

 This seems fine, after a bunch of testing to make sure.

>      """Takes a search string from the cli for Search or Provides
>         and cleans it up so it doesn't make us vomit"""
>      
> -    if re.match('.*[\*,\[,\],\{,\},\?,\+].*', arg):
> +    if re.search('[*[\]{}?+]', arg):
>          restring = fnmatch.translate(arg)
>      else:
>          restring = re.escape(arg)

 I always assumed the commas were a typo, or brain fart or something,
and AKAICS there are no provides have have a comma in them ... but one
reason I never did anything is that a comma would take a different
branch now and AFAIK there are no callers of refineSearchPattern.

>              for line in content:
> -                if re.match('^\s*\#.*', line) or re.match('^\s*$', line):
> +                if re.match('\s*(#|$)', line):
>                      continue
>                  mirror = line.rstrip() # no more trailing \n's
>                  mirror = mirror.replace('$ARCH', '$BASEARCH')
> @@ -1848,7 +1848,7 @@ def getMirrorList(mirrorlist, pdict = None):
>      if fo is not None:
>          content = fo.readlines()
>          for line in content:
> -            if re.match('^\s*\#.*', line) or re.match('^\s*$', line):
> +            if re.match('\s*(#|$)', line):
>                  continue
>              mirror = line.rstrip() # no more trailing \n's
>              mirror = mirror.replace('$ARCH', '$BASEARCH')

 The whole duped code in two places thing makes me twitch, and the lack
of compile is notable, but that's not your fault ... and the change
seems fine.



More information about the Yum-devel mailing list