Message summary subjects #1589

kbingham · 2021-10-20T12:45:33Z

This series tackles presenting threads where a multi-patch series is sent with a cover letter, for instance when sent with git-send-mail.

The default presentation in the thread view hides the details required to identify the patch titles:

With this development the individual patches can be seen to show their titles on the summary.

To prevent adding a lot of noise, replies do not further repeat the message title. The implemenation here so far is just to check if 'subject.startswith(('Re:')) and ignore those ones which does not seem fool-proof (and may be entirely locale dependant based upon the senders) so I'm looking for advice on a better way to implement that.

I had wondered if comparing the Subject against a parent subject, and only printing if they differ would be better, but the prefixes that get added by replies would still affect that too.

kbingham · 2021-10-20T13:00:23Z

Just discovered:
mjg@f788ddd
Which looks like a better way to implement the get_subject() perhaps.

pazz · 2021-10-20T13:39:37Z

I'm not sure which implementation of "get_subject" is better; perhaps @mjg has an opinion. I am actually surprised that this isn't already there (is that patch you mention not in master?).

Ideally, the message summary would be configurable in the theme just like threadline is in search mode.
At the very least I believe we should have config settings to enable/disable whether subjects are displayed at all and if so, if they are re-interpreted as you suggest.

kbingham · 2021-10-20T13:57:56Z

I'm not running master yet, still on 0.9.1 as I couldn't (yet) figure out how to get the notmuch2 bindnigs to work.
But probably time to retry that and get back on master.

That said, no @mjg 's patch isn't on master, probably for the same reasons you've highlighted above ;-) (the get_subject and summary line is all in one patch currently.).

Once I'm on master, I'll see about sending a PR for just the get_subject() as that is likely to be useful on it's own anyway, and then tackle the configuration file parts.

mjg · 2021-10-20T13:58:58Z

I use alot with a couple of private additions, and per-message-subject is one of them. Similarly, unset hide_thread_subject was one of my first settings when I (partially) switched to (neo)mutt.

I don't think matching for Re: is a good way forward: It depends on the sender's locale and it wrongly matches on changed subjects such as Re: Your subject IMPORTANT CORRECTION. Similarly, the option to check whether the subject ends in the original subject fails on IMPORTANT Correction. Re: Your subject and such.

But then I don't know why anyone would want to hide subjects, unless it's on a narrow screen or with deeply nested threads. In other words: I'm the wrong person to ask ;)

Maybe a config which can be bound to a key?

BTW: Some of my old PRs might be stale, i.e. would possibly need rework, especially everything related to "full thread", "requery". Whenever there are recent rebases it means that I'm at least still using them, such as per-message-subject.

mjg · 2021-10-20T14:05:33Z

... and now I noticed the real content of the question...

So, deferring decode_header() like @kbingham does is certainly a good idea unless you expect to call get_subject() anyways (which i did). OTOH one should probably catch the exceptions like in my patch. Other than that we're doing the same thing here, aren't we?

kbingham · 2021-10-20T14:42:43Z

More or less the same thing yes.
And how to determine when to trim(stop adding) the subject is the hard part that I don't yet know ;-)

But I think it's important (to me) as otherwise the screen is just too busy in review conversations:
So I think this:

Is much cleaner/more readable than

I think I'd like to figure out how to change the colour/highlight of the subject too, like it does on the main search view so that we can quickly differentiate between the sender/datestamp/subject on the summary line.

More for me to experiment and play with I guess.

pazz, do you still use the IRC channel? or just lurk - I've asked a question there about trying to run on master, so I can't rebase these to latest yet.

kbingham · 2021-10-21T12:50:04Z

alot/widgets/thread.py

+        # Hide Subjects which are replies to the parent message.
+        # This allows threads containing multiple messages with distinct
+        # subjects to be displayed without adding noise to the replies.
+        if not subject.startswith(('Re:')):


And already I've hit RE: which fails on this filter of course.
Any ideas on a good way to detect that this is an actual reply (and not a separate conversation posted as an in-reply-to) are welcome.

mjg · 2021-10-21T13:11:15Z

The thing is that git send-email and some people do the "wrong thing": they specify both In-Reply-To and References when indeed the patches are not a reply to the cover letter, for example. OTOH I don't think there's a widely accepted standard for saying "related but not a reply".

Just for fun: A bank which I sent an e-mail "Re: some topic" to apparantly translated "Re" into "Reklamation" (complaint) automatically and redirected the e-mail to that department...

So, I think your best option to check whether e-mail 2 is a "real reply" to e-mail 1 is by checking whether e-mail 1's subject is contained in that of e-mail 2 (plus maybe a safety check on difference of length being smallish). This catches all kinds of "Re/RE/AW", and forwards typically do not set the headers, so they're not in the mix anyways.

For an extra safety measure, replace the matched subject by ..., so that message 2 would display the subject as Re: ... or AW: ... or Why do you write an e-mail with subject ... to me? which loses none of the extra information but still is much shorter than the full subject. I think even I would like that.

Or let the user define a hook or regex for the match.

kbingham · 2021-10-21T13:16:05Z

Oh, I really like the 'replace' with '...' and that solves a lot of the concerns anyway, and provides the same end goal.

kbingham · 2021-10-21T17:00:57Z

@mjg The '...' looks nice and seems to solve it so far.
I'll keep testing this locally for a bit and fix up the warning, and some more cleanups.

I still need some help getting the master branch to run before I can base this on top of 0.10 though.
It seems like the failure starts after the notmuch2 API change, but as far as I can tell I have the notmuch2 bindings installed.

kbingham · 2021-10-22T12:17:09Z

@mjg Thanks for that suggestion, it's working really well so far, and helps highlight differences in the subject from the parent quite nicely.

There are a few 'exceptions' I might want to look into (mostly so far between case sensitivity of Re: vs RE: ) - but it's handling the main use cases really well.

mjg · 2021-10-22T13:15:23Z

Interesting. I think what I meant was slightly different, but both works. The differece is in whether you replace a match for the original subject (if it matches) or for the previous subject, or in other words: When does the meaning of ... change? Given:

Really important topic
+Re: Really important topic
++Re: Really important topic
+++AW: Re: Really important topic
++++Re: AW: Re: Really important topic
+++++Re: AW: Re: Really important topic

you translate this (if I'm not mistaken) into

Really important topic
+Re: ...
++...
+++AW: ...
++++Re: ...
+++++...

and I was more thinking along the lines of:

Really important topic
+Re: ...
++Re: ...
+++AW: Re: ...
++++Re: AW: Re: ...
+++++Re: AW: Re:...

I.e. keep the meaning of ... as long as it matches. I haven't looked at the code, though.

kbingham · 2021-10-22T13:23:13Z

The difficulty is that I can /only/ match against a single parent. I can't match that 'all the way up'.
Otherwise that would only ever consider the top level thread subject.

But more or less, I'm simply replacing the parent subject in the current subject ..
So this indeed would be:

Really important topic
+Re: ...        ### Matches top thread
++ ...          ### No additional Re: so matches the above and all removed.
+++AW: ...      ### Only the AW: is 'new' and visible.
++++Re: ...     ### The AW was in parent, and now also removed.
+++++...        ### Entire subject is the same as parent.

To keep all prefixes we'd have to be able to know that we could selectively remove 'standard' prefixes from the match criteria, which might be reasonable to do.

We could 'strip' the list of prefixes (https://en.wikipedia.org/wiki/List_of_email_subject_abbreviations) before making the match/compares?

kbingham · 2021-10-22T13:35:50Z

To clarify in:

If I always match only the top level message summary - It would always try to replace "[PATCH 00/11] Document all the IPU3 IPA classes" which isn't actually replied to at all in the entire thread.

Here's the raw subjects for the first few lines of the image above:

[PATCH 00/11] Document all the IPU3 IPA classes
-> [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> -> Re: [libcamera-devel] [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> -> -> Re: [libcamera-devel] [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> -> -> Re: [libcamera-devel] [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> -> -> -> Re: [libcamera-devel] [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> [libcamera-devel] [PATCH 02/11] ipa: ipu3: Improve the documentation of BDS grid 
-> -> Re: [libcamera-devel] [PATCH 02/11] ipa: ipu3: Improve the documentation of BDS grid

Which currently generates:

[PATCH 00/11] Document all the IPU3 IPA classes
-> [PATCH 01/11] ipa: ipu3: Document IPAIPU3 class interface
-> -> Re: [libcamera-devel] ...
-> -> -> ...
-> -> -> ...
-> -> -> -> ...
-> [libcamera-devel] [PATCH 02/11] ipa: ipu3: Improve the documentation of BDS grid 
-> -> Re: ...

Note how the [libcamera-devel] is only present on patches that I received through the list, and differs to the subject of the mail that I may receive if the mail comes directly to me. It might be a race or specific to whether I was on direct CC/To as to which one gets used. I usually interpret mails that don't have the list on as being sent 'specifically' to me.

mjg · 2021-10-22T14:13:21Z

Sure, what I meant is: Starting at root, set threadsubject = subject of root. Then (pseudocode) for each child repeat:

if threadsubject in currentsubject:
    replace threadsubject by ... in currentsubject for display
else:
   threadsubject = currentsubject # for the subthread beginning here

As I wrote, it depends on how you actually interpret .... In your algorithm, messages with the same subject end up being represented differently: Re: ... vs. ... for the first reply vs. the reply to that. Now, the first reply clearly changed the subject (prepending Re: ) while the reply to that did not, so I guess by looking at it that way everything is fine with your approach.

kbingham · 2021-10-22T15:38:51Z

Sure, what I meant is: Starting at root, set threadsubject = subject of root. Then (pseudocode) for each child repeat:
if threadsubject in currentsubject:
    replace threadsubject by ... in currentsubject for display
else:
   threadsubject = currentsubject # for the subthread beginning here
As I wrote, it depends on how you actually interpret .... In your algorithm, messages with the same subject end up being represented differently: Re: ... vs. ... for the first reply vs. the reply to that. Now, the first reply clearly changed the subject (prepending Re: ) while the reply to that did not, so I guess by looking at it that way everything is fine with your approach.

Aha, ok that makes it clearer what you were intending actually, thanks. I'm not yet sure how easy it would be to implement that in the widget, I'm referencing 'up' by checking the parent of each summary, rather than iterating down ...

(Well, the iteration down happens when creating the Messages, but I don't think we should be trailing other message's subjects around with a Message object).

kbingham · 2021-11-12T10:26:42Z

Now that I've fixed alot to run on master with my config, I've rebased this. Probably still a bit more refinement needed, but this really helps my work-load a lot.

Provide a method to get the subject of a given message. The subject is cached by not-much in the database, and is efficient to access directly through the notmuch2.msg object. Store the subject in our Message representation so it can be used efficiently when parsing and displaying mail threads. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Fix no subject mails

Allow Message instances to be created with a reference to their parent Message object. Add a get_parent() helper to return the parent of a message, or None if it is a top level message. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Provide a reference to the parent of any Message when it is initialised. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Replies to a parent message are not shown to avoid filling the buffer with noisy repeating content. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Threads that do not have a subject will cause a LookupError() when retrieving the header. Catch this exception and set the subject to a null string. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

kbingham · 2021-12-03T00:01:51Z

Small updates here. The method of getting the subject has been simplified, as the subject headers are indexed by not-much - so from my understanding it's more efficient to simply get it when constructing the Message object, and retain it at that point.

I've also picked up a fix in this series, while testing as I sent myself a mail with no subject to validate this. And that hit a failure in existing code paths. So that's included as the last patch.

Open questions are, if this implementation for stripping the parent subject is satisfactory to all, and if this should be any kind of a config option... or if this should be default behaviour.

kbingham force-pushed the MessageSummarySubjects branch from 47df9e4 to 99f1b2f Compare October 20, 2021 12:45

kbingham marked this pull request as ready for review October 20, 2021 12:55

kbingham commented Oct 21, 2021

View reviewed changes

kbingham force-pushed the MessageSummarySubjects branch from 99f1b2f to 5172115 Compare October 21, 2021 16:58

kbingham force-pushed the MessageSummarySubjects branch from 5172115 to 5a9ac04 Compare October 22, 2021 12:20

kbingham force-pushed the MessageSummarySubjects branch from 5a9ac04 to dc14b15 Compare November 12, 2021 10:25

kbingham force-pushed the MessageSummarySubjects branch from dc14b15 to c4967e4 Compare November 12, 2021 10:28

kbingham force-pushed the MessageSummarySubjects branch from c4967e4 to 1d0f918 Compare December 2, 2021 23:17

kbingham added 4 commits December 2, 2021 23:20

db: thread: Initialise Messages with their parent

57b8c03

Provide a reference to the parent of any Message when it is initialised. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

widgets: thread: Include Subject in MessageSummary

46e7581

Replies to a parent message are not shown to avoid filling the buffer with noisy repeating content. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

kbingham force-pushed the MessageSummarySubjects branch from 1d0f918 to 317f6b3 Compare December 2, 2021 23:20

db: thread: Fix LookupError when no header(subject)

f7bb7b6

Threads that do not have a subject will cause a LookupError() when retrieving the header. Catch this exception and set the subject to a null string. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

kbingham force-pushed the MessageSummarySubjects branch from 317f6b3 to f7bb7b6 Compare December 2, 2021 23:55

kbingham changed the title ~~(RFC) Message summary subjects~~ Message summary subjects Jan 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Message summary subjects #1589

Message summary subjects #1589

kbingham commented Oct 20, 2021

kbingham commented Oct 20, 2021

pazz commented Oct 20, 2021

kbingham commented Oct 20, 2021

mjg commented Oct 20, 2021

mjg commented Oct 20, 2021

kbingham commented Oct 20, 2021

kbingham Oct 21, 2021

mjg commented Oct 21, 2021 •

edited

Loading

kbingham commented Oct 21, 2021

kbingham commented Oct 21, 2021

kbingham commented Oct 22, 2021

mjg commented Oct 22, 2021

kbingham commented Oct 22, 2021 •

edited

Loading

kbingham commented Oct 22, 2021

mjg commented Oct 22, 2021

kbingham commented Oct 22, 2021

kbingham commented Nov 12, 2021

kbingham commented Dec 3, 2021

Message summary subjects #1589

Are you sure you want to change the base?

Message summary subjects #1589

Conversation

kbingham commented Oct 20, 2021

kbingham commented Oct 20, 2021

pazz commented Oct 20, 2021

kbingham commented Oct 20, 2021

mjg commented Oct 20, 2021

mjg commented Oct 20, 2021

kbingham commented Oct 20, 2021

kbingham Oct 21, 2021

Choose a reason for hiding this comment

mjg commented Oct 21, 2021 • edited Loading

kbingham commented Oct 21, 2021

kbingham commented Oct 21, 2021

kbingham commented Oct 22, 2021

mjg commented Oct 22, 2021

kbingham commented Oct 22, 2021 • edited Loading

kbingham commented Oct 22, 2021

mjg commented Oct 22, 2021

kbingham commented Oct 22, 2021

kbingham commented Nov 12, 2021

kbingham commented Dec 3, 2021

mjg commented Oct 21, 2021 •

edited

Loading

kbingham commented Oct 22, 2021 •

edited

Loading