Added is:<flags> search predicate

[talneoran]

Pull request checklist:

• CHANGELOG.md was updated, if applicable

Description

This PR adds a search predicate that allows filtering messages based on some of their flags.
Usage: is:<flags> where flags is a comma seperated list of one of the following currently supported options:

• deleted: Checks for MessageFlag::Disabled
• sub or subscription: Checks for MessageFlag::Subscription
• timeout: Checks for MessageFlag::Timeout
• highlighted: Checks for MessageFlag::Highlighted
• system: Checks for MessageFlag::System

Examples:
is:deleted would show deleted (disabled) messages
is:sub,timeout would show messages that are either a subscription message or a timeout message

Obviously if this is added I'm open to changes and additions to the current options and their naming (these are just the options that I initially thought of given the available flags), as well as changes to the phrasing of the changelog entry.

Closes #2653

[leon-richardt]

Looks good so far, just asking for some documentation changes for now. 😄 Will compile and properly test later today

[leon-richardt]

One thing I found while testing: if you provide multiple is:<flags> predicates, the flags will be OR'ed over all predicates provided. Ideally, only the flags separated by commas should be OR'ed and separate predicates should be AND'ed. I believe this could be fixed by creating a separate QStringList flags for every new is:<flags> keyword (or by clearing the list before parsing a new predicate).

For illustration, here is an example. In the last screenshot, I would expect to only see messages that are deleted and highlighted. Instead, messages that are deleted or highlighted are shown:

Screenshots

[talneoran]

@leon-richardt This is actually something I noticed and is also true for the from:users and in:channels predicates in the current release (I implemented in:channels and this predicate out of from:users to be consistent so that's why).

I could fix it for all 3 like you said by creating a new predicate and appending it to the predicate list every time the keyword is parsed (instead of just once for all occurences), and that would also make the outer loop in the constructors for all predicates useless.
Question is since it applies to multiple predicates do I fix it in a seperate PR and only remove the loop in this one? Or maybe it can also be fixed per predicate by keeping multiple inner lists or something.

[leon-richardt]

@leon-richardt This is actually something I noticed and is also true for the from:users and in:channels predicates in the current release (I implemented in:channels and this predicate out of from:users to be consistent so that's why).

I'm not convinced these are 100% comparable. Any given message can only have exactly one author and only appear in exactly one channel while it can have multiple MessageFlags. However, you do bring up a perfectly valid point about consistency.

---

Question is since it applies to multiple predicates do I fix it in a seperate PR and only remove the loop in this one?

I think existing behavior should be changed in a separate PR.

[talneoran]

I'm not convinced these are 100% comparable. Any given message can only have exactly one author and only appear in exactly one channel while it can have multiple MessageFlags.

Right I see what you mean now, maybe it is better then to change it specifically for this predicate (and potentially others like it in the future), since there's no meaning to an AND relation for the other existing ones, and no reason to add seperate list entries for those.

[zneix]

Just tested the PR, few things

1. Just noticed that all the sub messages also get MessageFlag::Highlighted flag, thus they also show up when you search for highlighted messages:
   

2. Also I'm not sure if we want to change timeout/ban messages having MessageFlag::System, so your search could be a bit more specific.
   

I believe those things should/could be fixed before merging, but maybe in another PR.

[talneoran]

1. Just noticed that all the sub messages also get MessageFlag::Highlighted flag, thus they also show up when you search for highlighted messages:

Isn't this because of this setting?

Assuming it is, why is it wrong that is:highlighted captures highlighted sub messages when that highlight is enabled?

2. Also I'm not sure if we want to change timeout/ban messages having MessageFlag::System, so your search could be a bit more specific.

Not sure what you are suggesting, the filter still just finds messages with MessageFlag::System on. Maybe the meaning of the flag can be changed or maybe this flag shouldn't be supported in this filter, but otherwise do you think there's something worth changing with the implementation in this PR?

[zneix]

Isn't this because of this setting?
image

Assuming it is, why is it wrong that is:highlighted captures highlighted sub messages when that highlight is enabled?

You're right, I had it enabled - totally forgot that existed. Since it's a setting I guess it can stay this way. You don't have to change anything then I think - I just found that just a little confusing that sub messages come up when I search for highlights.

Not sure what you are suggesting, the filter still just finds messages with MessageFlag::System on.

Again, you're right 👍

Maybe the meaning of the flag can be changed or maybe this flag shouldn't be supported in this filter.

Yes, I was thinking that messages having MessageFlag::Timeout could be filtered / removed when searching with predicate is:system, so those won't show up.

but otherwise do you think there's something worth changing with the implementation in this PR?

I don't think you should change how / which flags are assigned to messages since that's just a little out of scope of this Pull Request. If anything, try to play around with the point I specified above.

[pajlada]

Tested and works as expected, looks good to me.
Thanks for your contribution!