Re: [PATCH v2] Name thread based on matching msgs instead of first msg.

[Carl Worth]

On Sat, 17 Apr 2010 13:59:22 -0400, Jesse Rosenthal  wrote:
> At the moment all threads are named based on the name of the first message
> in the thread. However, this can cause problems if people either start
> new threads by replying-all (as unfortunately, many out there do) or
> change the subject of their mails to reflect a shift in a thread on a
> list.

Thanks, Jesse. This is a nice new feature.

I've pushed this out now, followed up with several changes:

  * Simplify the assignment of the correct subject
  * Add test cases
  * Fix the feature to actually pass the tests

I know that my simplified version was broken (the tests showed
that). I'm not sure if the original version would have passed the tests
or not. But at least things seem to be working now.

-Carl


pgpWvdGCrWksi.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2] Name thread based on matching msgs instead of first msg.

[Carl Worth]

On Sat, 17 Apr 2010 13:59:22 -0400, Jesse Rosenthal  
wrote:
> At the moment all threads are named based on the name of the first message
> in the thread. However, this can cause problems if people either start
> new threads by replying-all (as unfortunately, many out there do) or
> change the subject of their mails to reflect a shift in a thread on a
> list.

Thanks, Jesse. This is a nice new feature.

I've pushed this out now, followed up with several changes:

  * Simplify the assignment of the correct subject
  * Add test cases
  * Fix the feature to actually pass the tests

I know that my simplified version was broken (the tests showed
that). I'm not sure if the original version would have passed the tests
or not. But at least things seem to be working now.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[PATCH v2] Name thread based on matching msgs instead of first msg.

[Jesse Rosenthal]

At the moment all threads are named based on the name of the first message
in the thread. However, this can cause problems if people either start
new threads by replying-all (as unfortunately, many out there do) or
change the subject of their mails to reflect a shift in a thread on a
list.

This patch names threads based on (a) matches for the query, and (b) the
search order. If the search order is oldest-first (as in the default
inbox) it chooses the oldest matching message as the subject. If the
search order is newest-first it chooses the newest one.

Reply prefixes ("Re: ", "Aw: ", "Sv: ", "Vs: ") are ignored
(case-insensitively) so a Re: won't change the subject.

Note that this adds a "sort" argument to _notmuch_thread_create and
_thread_add_matched_message, so that when constructing the thread we can
be aware of the sort order.

Signed-off-by: Jesse Rosenthal 
---
This patch is rebased against current master, and offered for inclusion
in 0.3.

This patch is necessary for me to keep track of my mail, based on the
emailing habits of my correspondents. If others much prefer the original
behavior, I would still request that this feature be a configurable
option, even if not the default.

 lib/notmuch-private.h |3 ++-
 lib/query.cc  |3 ++-
 lib/thread.cc |   32 
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index d52d84d..94cce1b 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -205,7 +205,8 @@ notmuch_thread_t *
 _notmuch_thread_create (void *ctx,
notmuch_database_t *notmuch,
const char *thread_id,
-   const char *query_string);
+   const char *query_string,
+   notmuch_sort_t sort);

 /* message.cc */

diff --git a/lib/query.cc b/lib/query.cc
index 10f8dc8..3e20f59 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -299,7 +299,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
 return _notmuch_thread_create (threads->query,
   threads->query->notmuch,
   threads->thread_id,
-  threads->query->query_string);
+  threads->query->query_string,
+  threads->query->sort);
 }

 void
diff --git a/lib/thread.cc b/lib/thread.cc
index 3aa9d48..5bf8354 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -129,7 +129,8 @@ _thread_add_message (notmuch_thread_t *thread,

 static void
 _thread_add_matched_message (notmuch_thread_t *thread,
-notmuch_message_t *message)
+notmuch_message_t *message,
+notmuch_sort_t sort)
 {
 time_t date;
 notmuch_message_t *hashed_message;
@@ -142,6 +143,28 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 if (date > thread->newest || ! thread->matched_messages)
thread->newest = date;

+const char *subject;
+const char *cleaned_subject;
+
+subject = notmuch_message_get_header (message, "subject");
+
+if ((strncasecmp (subject, "Re: ", 4) == 0) ||
+   (strncasecmp (subject, "Aw: ", 4) == 0) ||
+   (strncasecmp (subject, "Vs: ", 4) == 0) ||
+   (strncasecmp (subject, "Sv: ", 4) == 0)) {
+
+   cleaned_subject = talloc_strndup (thread,
+ subject + 4,
+ strlen(subject) - 4);
+} else {
+   cleaned_subject = talloc_strdup (thread, subject);
+}
+
+if ((sort == NOTMUCH_SORT_OLDEST_FIRST && date <= thread->newest) ||
+   (sort != NOTMUCH_SORT_OLDEST_FIRST && date == thread->newest)) {
+   thread->subject = talloc_strdup (thread, cleaned_subject);
+}
+
 thread->matched_messages++;

 if (g_hash_table_lookup_extended (thread->message_hash,
@@ -209,7 +232,8 @@ notmuch_thread_t *
 _notmuch_thread_create (void *ctx,
notmuch_database_t *notmuch,
const char *thread_id,
-   const char *query_string)
+   const char *query_string,
+   notmuch_sort_t sort)
 {
 notmuch_thread_t *thread;
 const char *thread_id_query_string;
@@ -296,7 +320,7 @@ _notmuch_thread_create (void *ctx,
_thread_add_message (thread, message);

if (! matched_is_subset_of_thread)
-   _thread_add_matched_message (thread, message);
+   _thread_add_matched_message (thread, message, sort);

_notmuch_message_close (message);
 }
@@ -323,7 +347,7 @@ _notmuch_thread_create (void *ctx,
 notmuch_messages_move_to_next (messages))
{
message = notmuch_messages_get (messages);
-   _thread_add_matched_message (thread, message);
+   _thread_add_matched_message (thread, message, sort);
_

[PATCH v2] Name thread based on matching msgs instead of first msg.

[Jesse Rosenthal]

At the moment all threads are named based on the name of the first message
in the thread. However, this can cause problems if people either start
new threads by replying-all (as unfortunately, many out there do) or
change the subject of their mails to reflect a shift in a thread on a
list.

This patch names threads based on (a) matches for the query, and (b) the
search order. If the search order is oldest-first (as in the default
inbox) it chooses the oldest matching message as the subject. If the
search order is newest-first it chooses the newest one.

Reply prefixes ("Re: ", "Aw: ", "Sv: ", "Vs: ") are ignored
(case-insensitively) so a Re: won't change the subject.

Note that this adds a "sort" argument to _notmuch_thread_create and
_thread_add_matched_message, so that when constructing the thread we can
be aware of the sort order.

Signed-off-by: Jesse Rosenthal 
---
This patch is rebased against current master, and offered for inclusion
in 0.3.

This patch is necessary for me to keep track of my mail, based on the
emailing habits of my correspondents. If others much prefer the original
behavior, I would still request that this feature be a configurable
option, even if not the default.

 lib/notmuch-private.h |3 ++-
 lib/query.cc  |3 ++-
 lib/thread.cc |   32 
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index d52d84d..94cce1b 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -205,7 +205,8 @@ notmuch_thread_t *
 _notmuch_thread_create (void *ctx,
notmuch_database_t *notmuch,
const char *thread_id,
-   const char *query_string);
+   const char *query_string,
+   notmuch_sort_t sort);
 
 /* message.cc */
 
diff --git a/lib/query.cc b/lib/query.cc
index 10f8dc8..3e20f59 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -299,7 +299,8 @@ notmuch_threads_get (notmuch_threads_t *threads)
 return _notmuch_thread_create (threads->query,
   threads->query->notmuch,
   threads->thread_id,
-  threads->query->query_string);
+  threads->query->query_string,
+  threads->query->sort);
 }
 
 void
diff --git a/lib/thread.cc b/lib/thread.cc
index 3aa9d48..5bf8354 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -129,7 +129,8 @@ _thread_add_message (notmuch_thread_t *thread,
 
 static void
 _thread_add_matched_message (notmuch_thread_t *thread,
-notmuch_message_t *message)
+notmuch_message_t *message,
+notmuch_sort_t sort)
 {
 time_t date;
 notmuch_message_t *hashed_message;
@@ -142,6 +143,28 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 if (date > thread->newest || ! thread->matched_messages)
thread->newest = date;
 
+const char *subject;
+const char *cleaned_subject;
+
+subject = notmuch_message_get_header (message, "subject");
+
+if ((strncasecmp (subject, "Re: ", 4) == 0) ||
+   (strncasecmp (subject, "Aw: ", 4) == 0) ||
+   (strncasecmp (subject, "Vs: ", 4) == 0) ||
+   (strncasecmp (subject, "Sv: ", 4) == 0)) {
+
+   cleaned_subject = talloc_strndup (thread,
+ subject + 4,
+ strlen(subject) - 4);
+} else {
+   cleaned_subject = talloc_strdup (thread, subject);
+}
+
+if ((sort == NOTMUCH_SORT_OLDEST_FIRST && date <= thread->newest) ||
+   (sort != NOTMUCH_SORT_OLDEST_FIRST && date == thread->newest)) {
+   thread->subject = talloc_strdup (thread, cleaned_subject);
+}
+
 thread->matched_messages++;
 
 if (g_hash_table_lookup_extended (thread->message_hash,
@@ -209,7 +232,8 @@ notmuch_thread_t *
 _notmuch_thread_create (void *ctx,
notmuch_database_t *notmuch,
const char *thread_id,
-   const char *query_string)
+   const char *query_string,
+   notmuch_sort_t sort)
 {
 notmuch_thread_t *thread;
 const char *thread_id_query_string;
@@ -296,7 +320,7 @@ _notmuch_thread_create (void *ctx,
_thread_add_message (thread, message);
 
if (! matched_is_subset_of_thread)
-   _thread_add_matched_message (thread, message);
+   _thread_add_matched_message (thread, message, sort);
 
_notmuch_message_close (message);
 }
@@ -323,7 +347,7 @@ _notmuch_thread_create (void *ctx,
 notmuch_messages_move_to_next (messages))
{
message = notmuch_messages_get (messages);
-   _thread_add_matched_message (thread, message);
+   _thread_add_matched_message (thread, message, sort);