#3144: Parent and child match pattern modifier
--------------------------+----------------------
Reporter: jlh | Owner: mutt-dev
Type: enhancement | Status: new
Priority: minor | Milestone:
Component: mutt | Version:
Resolution: | Keywords:
--------------------------+----------------------
Comment (by kevin8t8):
Sorry but I am not willing to apply this in its current form.
The behavior of "not" (!) is unclear and even contradictory in the patch.
Should it apply to the pattern or to the parent/child operator? Should
there be a difference between
{{{
!>~f foo.com
!>(~f foo.com)
!(>~f foo.com)
>!~f foo.com
>(!~f foo.com)
}}}
Right now it looks like it's always applied to the pattern.
Related is the behavior when a message doesn't have a parent or child. It
looks like the code is trying to use the "pattern not" to make this
decision, but I'm not convinced that's always appropriate.
In the childsmatch logic in mutt_pattern_exec():
{{{
+ for (; t; t = t->next) {
+ if (!t->message)
+ continue;
+ if (pattern_exec (pat, flags, ctx, t->message, cache))
+ return !pat->not;
+ }
+ return pat->not;
}}}
The not is already evaluated in pattern_exec() but then it's returning
!pat->not. If the pattern were ">!~f bar", this will return false if one
of the children is not from bar (i.e. matches the pattern). After the
loop it's returning pat->not. With the same pattern ">!~f bar", it will
return true if _all_ of the children messages were from bar (i.e. none of
the children match the pattern).
The problem is the conflation of trying to apply "not" to the operator
logic and the pattern at the same time.
Other notes:
* Does it make sense for a parent/child operator to be applied to ~()
operator? Right now the code is, but this seems a bad idea.
* At an implementation level, I'm not happy with how the patch is
structured. It's not aligned with the rest of the pattern code. Creating
a new outer mutt_pattern_exec() that just deals with this pseudo-operator
is ugly.
* Vincent: an important correction to your version of the patch. Note
that the pattern_cache_t applies to a single header: it doesn't deal with
threads. So inside the mutt_pattern_exec(), the first two calls to
pattern_exec() should pass NULL for the cache because they are looking at
parent/children. Only the third call should actually pass the cache
through.
--
Ticket URL: <https://dev.mutt.org/trac/ticket/3144#comment:11>
Mutt <http://www.mutt.org/>
The Mutt mail user agent
