[master] [PATCH] Refactor and cleanup our localization module

Vratislav Podzimek vpodzime at redhat.com
Thu Jan 10 09:48:03 UTC 2013


On Wed, 2013-01-09 at 06:36 -0800, Brian C. Lane wrote:
> On Wed, Jan 09, 2013 at 02:33:17PM +0100, Vratislav Podzimek wrote:
> > On Tue, 2013-01-08 at 09:22 -0800, Brian C. Lane wrote:
> > > On Tue, Jan 08, 2013 at 01:03:27PM +0100, Vratislav Podzimek wrote:
> > > > expand_langs may use parse_langcode which should be a global function
> > > > not a static method of the Language class. Also some pieces deserve
> > > > better documentation and some other pieces were potentially dangerous.
> > > > 
> > > > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > > > ---
> > > >  pyanaconda/localization.py | 91 +++++++++++++++++++++++++++-------------------
> > > >  1 file changed, 53 insertions(+), 38 deletions(-)
> > > 
> > > 
> > > > +
> > > > +    match = LANGCODE_RE.match(langcode)
> > > > +    return match.groupdict()
> > > 
> > > The match could fail. I'd catch that and return an empty {}
> > Good point, thanks! Fixing locally.
> > 
> > > 
> > > 
> > > > +    lang_dict = parse_langcode(astring)
> > > > +    base, loc, enc, script = [lang_dict[key] for key in ("language",
> > > > +                                      "territory", "codeset", "modifier")]
> > > 
> > > The key may not be in the lang_dict, add 'if key in lang_dict' to the end
> > > of the list comprehension.
> > The key has to be in dict if the dict is not {} (as suggested in your
> > previous comment). Or am I wrong? I think check for lang_dict != {}
> > should be enough. Using 'if key in lang_dict' doesn't make much sense to
> > me as it would return the items in a wrong order in case something is
> > missing.
> 
> I don't think we can guarantee that everything passed to parse_langcode
> will have all the right pieces. What does the regex do when only some of
> that matches? If it returns {} then sure, we don't need the extra check.
The RE is written in a way that only the first (lang) part is required.
The other parts are marked as optional and match.groupdict() puts None
as values for those missing parts. So one or more of loc, enc and script
may end up being None, but that's okay and shouldn't cause any troubles
in the code following that assignment.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list