[weboob] [PATCH 1/1] [piratebay] ported to browser2

Matthieu Weber mweber+weboob at free.fr
Fri Mar 18 12:52:51 CET 2016


Le ven. 18/03/2016 à 11:18:10 +0100, Florent écrivait:
> Changes on modules and on core must always be in separates patches (to help
> backporting of module patches, to have an easy to read history, etc). I can
> split it for you if you want.

I can do it myself, that's not difficult.

> >      @debug()
> >      def filter(self, el):
> >          if isinstance(el, (tuple, list)):
> >@@ -317,7 +321,10 @@ class RawText(Filter):
> >          if el.text is None:
> >              return self.default
> >          else:
> >-            return unicode(el.text)
> >+            if self.children:
> >+                return unicode(el.text_content())
> >+            else:
> >+                return unicode(el.text)
> >
> >
> 
> Minor: why not something like it:
> 
>    if el.text is None:
>        return self.default
> 
>    if self.children:
>        return unicode(el.text_content())
> 
>    return unicode(el.text)

Because it looks unbalanced. Because « el.text_content() » and
« el.text » are two alternative forms of the same behaviour
(extracting "THE text", for a definition of "THE text" that depends on
whether the user wants to read the children elements or not), so the
existence of these alternatves is more visible when code is symmetric.

And I'm starting to wonder if « if el.text is None: return
self.default » will not be triggered when applying « RawText('foo',
children=True) » to « <foo><bar/> some text</foo> » and fail to return
« "some text" ».

I'll try and write unit tests for it and see what goes.

Matthieu
-- 
 (~._.~)            Matthieu Weber - mweber at free.fr              (~._.~)
  ( ? )                http://weber.fi.eu.org/                    ( ? ) 
 ()- -()          public key id : 0x85CB340EFCD5E0B3             ()- -()
 (_)-(_) "Humor ist, wenn man trotzdem lacht (Otto J. Bierbaum)" (_)-(_)



More information about the weboob mailing list