[PATCH 2/5] Short-circuit layouts matching

Vratislav Podzimek vpodzime at redhat.com
Tue Nov 12 14:10:58 UTC 2013


On Tue, 2013-11-12 at 09:07 -0500, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Anne Mulhern" <amulhern at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> > Sent: Tuesday, November 12, 2013 8:35:09 AM
> > Subject: Re: [PATCH 2/5] Short-circuit layouts matching
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Martin Kolman" <mkolman at redhat.com>
> > > To: anaconda-patches at lists.fedorahosted.org
> > > Sent: Monday, November 11, 2013 12:14:05 PM
> > > Subject: [PATCH 2/5] Short-circuit layouts matching
> > > 
> > > From: Vratislav Podzimek <vpodzime at redhat.com>
> > > 
> > > If no string is entered, all layouts should match. Also move the
> > > have_word_match
> > > function to a better place making it reusable.
> > > 
> > > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > > ---
> > >  pyanaconda/iutil.py                  | 17 +++++++++++++++++
> > >  pyanaconda/ui/gui/spokes/keyboard.py | 23 ++++++-----------------
> > >  2 files changed, 23 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> > > index f13fb10..6cf9562 100644
> > > --- a/pyanaconda/iutil.py
> > > +++ b/pyanaconda/iutil.py
> > > @@ -769,3 +769,20 @@ def get_mount_paths(devnode):
> > >      majmin = "%d:%d" % (os.major(devno),os.minor(devno))
> > >      mountinfo = (line.split() for line in open("/proc/self/mountinfo"))
> > >      return [info[4] for info in mountinfo if info[2] == majmin]
> > > +
> > > +def have_word_match(str1, str2):
> > > +    """Tells if all words from str1 exist in str2 or not."""
> > > +
> > > +    if str1 is None or str2 is None:
> > > +        return False
> > > +
> > > +    str1 = str1.lower()
> > > +    str1_words = str1.split()
> > > +
> > > +    try:
> > > +        str2 = str2.lower()
> > > +        for word in str1_words:
> > > +            str2.index(word)
> > > +        return True
> > > +    except ValueError:
> > > +        return False
> > > diff --git a/pyanaconda/ui/gui/spokes/keyboard.py
> > > b/pyanaconda/ui/gui/spokes/keyboard.py
> > > index a42cf2b..42a7fee 100644
> > > --- a/pyanaconda/ui/gui/spokes/keyboard.py
> > > +++ b/pyanaconda/ui/gui/spokes/keyboard.py
> > > @@ -32,8 +32,10 @@ from pyanaconda.i18n import _, N_, CN_
> > >  from pyanaconda.constants import DEFAULT_KEYBOARD, THREAD_KEYBOARD_INIT,
> > >  THREAD_ADD_LAYOUTS_INIT
> > >  from pyanaconda.ui.communication import hubQ
> > >  from pyanaconda.threads import threadMgr, AnacondaThread
> > > +from pyanaconda.iutil import have_word_match
> > >  
> > >  import threading
> > > +import time
> > >  import locale as locale_mod
> > >  
> > >  import logging
> > > @@ -68,27 +70,14 @@ class AddLayoutDialog(GUIObject):
> > >          self._chosen_layouts = []
> > >  
> > >      def matches_entry(self, model, itr, user_data=None):
> > > -        def have_word_match(str1, str2):
> > > -            """Tells if all words from str1 exist in str2 or not."""
> > > -
> > > -            if str1 is None or str2 is None:
> > > -                return False
> > > -
> > > -            str1 = str1.lower()
> > > -            str1_words = str1.split()
> > > -
> > > -            try:
> > > -                str2 = str2.lower()
> > > -                for word in str1_words:
> > > -                    str2.index(word)
> > > -                return True
> > > -            except ValueError:
> > > -                return False
> > > +        entry_text = self._entry.get_text()
> > > +        if not entry_text:
> > > +            # everything matches empty string
> > > +            return True
> > >  
> > >          value = model[itr][0]
> > >          eng_value =
> > >          self._xkl_wrapper.get_layout_variant_description(value,
> > >          xlated=False)
> > >          xlated_value =
> > >          self._xkl_wrapper.get_layout_variant_description(value)
> > > -        entry_text = self._entry.get_text()
> > >  
> > >          return have_word_match(entry_text, eng_value) or
> > >          have_word_match(entry_text, xlated_value)
> > >  
> > > --
> > > 1.8.4.2
> > 
> > Perhaps it would be better to have have_word_match start with some lines as:
> > 
> > if not str1: return True
> > if not str2: return False
> > 
> > rather than changing the body of matches_entry.
> > 
> > not entry_text will be True if the string is None.
> > 
> > Also, maybe something like the below would be better than the loop +
> > exception in have_word_match:
> > 
> > str1_words = str1.lower().split()
> > str2 = str2.lower()
> > return all(map(lambda x: str2.find(x) != -1, str1_words))
> > 
> > - mulhern
> > 
> 
> From the tests it looks like you're not testing whether words from
> str1 match anywhere in str2, but whether all the words in str1 also
> occur as words in str2, in which case, instead of above three lines maybe:
> 
> return frozenset(str1.lower().split()) <= frozenset(str2.lower.split())
That means I'd have to change the tests.

-- 
Vratislav Podzimek

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



More information about the anaconda-patches mailing list