Closed romanofski closed 6 years ago
Hey Fraser, cheers for the feedback. I had similar worries when I was stacking the constraints in purebred. Good to know that it doesn't go far with it's value. I'm reading from your comment, that the best way forward is to expose the new functionality as is, instead of wrapped in additional HasFoo
instances.
In regards to authors, I agree. I think I'll drop it for now and come back if we really need it.
I'll see how far a single NotmuchItem
works for handling it in a list widget and potentially in a tree.
I worked on it a bit more and I'm now convinced that we can't do without the authors. If you want to show any form of threading -> mails this information is vital to get a glimpse who's participating in a conversation. Unless we build our own threading views which I'm not keen on unless we can beat with something more "productive" than what's out there.
The thing tho is, I don't understand what you're trying to achieve with matched and unmatched authors. The notmuch documentation describes the function notmuch_thread_get_authors
as returning a "comma separated list of the names of the authors". There is not really much to parse other than maybe splitting on comma and giving back a list of Authors (which are maybe a type alias for Text
). If you have time to spare, mind to elaborate?
@romanofski from notmuch.h
:
/**
* Get the authors of 'thread' as a UTF-8 string.
*
* The returned string is a comma-separated list of the names of the
* authors of mail messages in the query results that belong to this
* thread.
*
* The string contains authors of messages matching the query first, then
* non-matched authors (with the two groups separated by '|'). Within
* each group, authors are ordered by date.
*
* The returned string belongs to 'thread' and as such, should not be
* modified by the caller and will only be valid for as long as the
* thread is valid, (which is until notmuch_thread_destroy or until
* the query from which it derived is destroyed).
*/
const char *
notmuch_thread_get_authors (notmuch_thread_t *thread);
Argl... learn to comprehend. Cheers.
Actually I'm just realizing that I wasn't too dumb to comprehend. I've got 39beeb2a7e0e61fe2a5983e7eef9efec78e1279e and what I have is this:
/* Get the authors of 'thread'
*
* The returned string is a comma-separated list of the names of the
* authors of mail messages in the query results that belong to this
* thread.
*
* The returned string belongs to 'thread' and as such, should not be
* modified by the caller and will only be valid for as long as the
* thread is valid, (which is until notmuch_thread_destroy or until
* the query from which it derived is destroyed).
*/
const char *
notmuch_thread_get_authors (notmuch_thread_t *thread);
Checking my remote I must have obtained a fork somewhere from github (??). So never mind I got the correct clone from notmuchmail.org now.
On Tue, Nov 07, 2017 at 12:51:17AM +0000, Roman Joost wrote:
Actually I'm just realizing that I wasn't too dumb to comprehend. I've got 39beeb2a7e0e61fe2a5983e7eef9efec78e1279e and what I have is this:
/* Get the authors of 'thread' * * The returned string is a comma-separated list of the names of the * authors of mail messages in the query results that belong to this * thread. * * The returned string belongs to 'thread' and as such, should not be * modified by the caller and will only be valid for as long as the * thread is valid, (which is until notmuch_thread_destroy or until * the query from which it derived is destroyed). */ const char * notmuch_thread_get_authors (notmuch_thread_t *thread);
Checking my remote I must have obtained a fork somewhere from github (??). So never mind I got the correct clone from notmuchmail.org now.
Could be a change in a recent version. Well let's just parse it into a
[Mailbox]
but be aware the separate could be a comma OR a pipe.
It's definitely a pipe. I can literally see it in my currently hacked up version of purebred. So I go with your comment.
Alright this has got a bunch of new thread functions. I've tested them with a WIP patch I currently have on my hard disk. So I guess you'll have to trust me that they work (and I'm an engineer (tm)).
Anyway, what I'm not sure about are the optics (and perhaps general style) in regards to threadAuthors
. I've left them in Binding.chs
which strikes me as the wrong place, but yeah.. please have a look and let me know what should be changed.
Change requests:
ThreadAuthors
bits to high-level moduleWas iffy about depending on text
but it is the correct thing. So all good there.
Followup task: check whether names like, Sir Bob Ratley, III
break the parsing.
Alright updated. The only outstanding thing is if Sir Bob Ratley, III
results in a parsing error... Might check it tomorrow if I have time. The way the code is written now it certainly would if not notmuch is doing something to the names ...
Hang on with the merging of this patch. I actually run into a segfault and it's introduced with this patch. Can't reliably reproduce it yet tho :(
Found the mail which crashes in correlation with calling threadAuthors
. I've changed the function to just return a ByteString
and it still crashes.
Not sure where to start looking since gdb's backtrace is very small.
I'll check tomorrow on another platform if I can reliably reproduce it there as well.
@romanofski did you try adding detachPtr
on those new functions returning strings, as I suggested?
[edited to add] you are unlikely to find out anything useful via gdb.
Alright updated. I've added the "workaround" to check for NULL
which fixes the segfault.
Alright, ACK from me then.
On Fri, Nov 24, 2017 at 08:28:46AM +0000, Roman Joost wrote:
romanofski commented on this pull request.
@@ -356,10 +358,19 @@ thread_get_messages ptr = liftIO $ withThread ptr $ \ptr' ->
= messagesToList . Messages
-- notmuch_thread_get_matched_messages -> Int --- notmuch_thread_get_authors -> String --- notmuch_thread_get_subject + +thread_get_authors :: Thread a -> IO (Maybe B.ByteString) +thread_get_authors ptr = withThread ptr $ \ptr' -> do
- r <- {#call thread_get_authors #} ptr'
- if r == nullPtr
- then pure Nothing
- else Just <$> B.packCString r
+thread_get_subject :: Thread a -> IO B.ByteString +thread_get_subject ptr = withThread ptr ({#call thread_get_subject #} >=> B.packCString)
yeah tried it out... empty string. Although checking the documentation again it says:
/** * Get the subject of 'thread' as a UTF-8 string. * * The subject is taken from the first message (according to the query * order---see notmuch_query_set_sort) in the query results that * belongs to this thread. * [...] */
-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/purebred-mua/hs-notmuch/pull/19#discussion_r152915818
So merge away I guess. I'll see to asking about the weird behavior for the thread authors on the mailing list.
This is currently an idea I'm trying out. I've added a few more missing functions and would continue to tinker with it. The motivation behind it is, that I can construct the same type for showing it in a list in purebred:
Not sure if I end up falling on my face down the road since I won't be able to distinguish mail from thread anymore.