Comment 1 for bug 2052936

Revision history for this message
Leonard Richardson (leonardr) wrote (last edit ):

First, let's take inventory of what we have in the 4.13 branch that's in the vicinity of this wishlist item:

ElementFilter.match(PageElement) -> bool
PageElement.match(Iterator[PageElement], ElementFilter) -> ResultSet

I'm OK with adding an Iterator that is the equivalent of ancestor-or-self. The name would probably be PageElement.self_and_parents.

    @property
    def self_and_parents(self) -> Iterator[PageElement]:
        yield self
        for i in self.parents:
            yield i

I don't want to add more find_* methods, but since we already have find_parents, I'm OK with adding an include_self argument to it.

A PageElement.matches() method that takes the find_* arguments and returns a boolean is a little troubling for two reasons. First, it's another method with a very complicated signature. But more importantly, you're probably not calling PageElement.matches() just once. You're probably iterating over elements somehow and calling matches() each time. If you do that you're creating an identical ElementFilter for every element in the iterator, which is incredibly inefficient. It'd be much better to build the ElementFilter once. And there is a solution that works that way: either pass the ElementFilter into one of the find_* methods, or use PageElement.match(). So I'm not really sold on PageElement.matches().

However, we probably want to rename PageElement.match() before releasing 4.13, since the name makes it look like it returns a boolean, like ElementFilter.match() does. Maybe it should be called PageElement.filter().