6f/398c9188318eaec8cef2b626d6ec993d79cba9

   1 Return-Path: <BATV+bfcd26d6da2a49b2fc9f+2418+infradead.org+hohndel@bombadil.srs.infradead.org>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 9B2234196F0\r
   6         for <notmuch@notmuchmail.org>; Tue,  6 Apr 2010 22:52:31 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -4.2\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5\r
  12         tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3] autolearn=ham\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id F21i+NaAOsCW for <notmuch@notmuchmail.org>;\r
  16         Tue,  6 Apr 2010 22:52:30 -0700 (PDT)\r
  17 Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 1BC29431FC1\r
  19         for <notmuch@notmuchmail.org>; Tue,  6 Apr 2010 22:52:30 -0700 (PDT)\r
  20 Received: from localhost ([::1] helo=localhost.localdomain)\r
  21         by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux))\r
  22         id 1NzOBh-0003IW-Ep\r
  23         for notmuch@notmuchmail.org; Wed, 07 Apr 2010 05:52:29 +0000\r
  24 Received: by localhost.localdomain (Postfix, from userid 500)\r
  25         id D52BCC007F; Tue,  6 Apr 2010 22:52:28 -0700 (PDT)\r
  26 From: Dirk Hohndel <hohndel@infradead.org>\r
  27 To: notmuch <notmuch@notmuchmail.org>\r
  28 Subject: [RFC] reordering and cleanup of thread authors\r
  29 Date: Tue, 06 Apr 2010 22:52:28 -0700\r
  30 Message-ID: <m31veru7vn.fsf@x200.gr8dns.org>\r
  31 MIME-Version: 1.0\r
  32 Content-Type: text/plain; charset=us-ascii\r
  33 X-SRS-Rewrite: SMTP reverse-path rewritten from <hohndel@infradead.org> by\r
  34         bombadil.infradead.org See http://www.infradead.org/rpr.html\r
  35 X-BeenThere: notmuch@notmuchmail.org\r
  36 X-Mailman-Version: 2.1.13\r
  37 Precedence: list\r
  38 List-Id: "Use and development of the notmuch mail system."\r
  39         <notmuch.notmuchmail.org>\r
  40 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  41         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  42 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  43 List-Post: <mailto:notmuch@notmuchmail.org>\r
  44 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  45 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  46         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  47 X-List-Received-Date: Wed, 07 Apr 2010 05:52:31 -0000\r
  48 \r
  49 \r
  50 This is based in part on some discussion on IRC today.\r
  51 When a thread is displayed in the search results, previously the authors\r
  52 were always displayed in chronological order. But if only part of the\r
  53 thread matches the query, that may or may not be the intuitive thing to\r
  54 do.\r
  55 Imagine the default "+inbox" query. Those mails in the thread that\r
  56 match the query are actually "new" (whatever that means). And some\r
  57 people seem to think that it would be much better to see those author\r
  58 names first. For example, imagine a long and drawn out thread that once\r
  59 was started by me; you have long read the older part of the thread and\r
  60 removed the inbox tag. Whenever a new email comes in on this thread, the\r
  61 author column in the search display will first show "Dirk Hohndel" - I\r
  62 think it should first show the actual author(s) of the new mail(s).\r
  63 \r
  64 The second cleanup that I've done in this context is to convert all\r
  65 author names into the much easier to parse "Firstname Lastname"\r
  66 format. Some broken mail systems (cough - Exchange - Cough) seem to\r
  67 prefer "Lastname, Firstname" - which makes the comma separated list of\r
  68 authors really hard to parse at one glance. Worse, it creates douplicate\r
  69 entries if a person happens to show in both orders. So this does a\r
  70 rather simplistic "look for a comma, if it's there, reorder" approach to\r
  71 cleaning up the author name.\r
  72 \r
  73 I don't think this is ready to be pulled. There was a strong request on\r
  74 IRC to make the re-ordering of the author names configurable. I think\r
  75 the cleanup of the names (First Last) should be configurable as well.\r
  76 And of course I wonder about my implementation, given that I'm still\r
  77 trying to wrap my mind around the current code base.\r
  78 \r
  79 Thanks\r
  80 \r
  81 /D\r
  82 \r
  83 diff --git a/lib/message.cc b/lib/message.cc\r
  84 index 721c9a6..ac0afd9 100644\r
  85 --- a/lib/message.cc\r
  86 +++ b/lib/message.cc\r
  87 @@ -35,6 +35,7 @@ struct _notmuch_message {\r
  88      char *thread_id;\r
  89      char *in_reply_to;\r
  90      char *filename;\r
  91 +    char *author;\r
  92      notmuch_message_file_t *message_file;\r
  93      notmuch_message_list_t *replies;\r
  94      unsigned long flags;\r
  95 @@ -110,6 +111,7 @@ _notmuch_message_create (const void *talloc_owner,\r
  96      message->in_reply_to = NULL;\r
  97      message->filename = NULL;\r
  98      message->message_file = NULL;\r
  99 +    message->author = NULL;\r
 100  \r
 101      message->replies = _notmuch_message_list_create (message);\r
 102      if (unlikely (message->replies == NULL)) {\r
 103 @@ -533,6 +535,20 @@ notmuch_message_get_tags (notmuch_message_t *message)\r
 104      return _notmuch_convert_tags(message, i, end);\r
 105  }\r
 106  \r
 107 +const char *\r
 108 +notmuch_message_get_author (notmuch_message_t *message)\r
 109 +{\r
 110 +    return message->author;\r
 111 +}\r
 112 +\r
 113 +void\r
 114 +notmuch_message_set_author (notmuch_message_t *message,\r
 115 +                           const char *author)\r
 116 +{\r
 117 +    message->author = talloc_strdup(message, author);\r
 118 +    return;\r
 119 +}\r
 120 +\r
 121  void\r
 122  _notmuch_message_set_date (notmuch_message_t *message,\r
 123                            const char *date)\r
 124 diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 125 index 88da078..79638bb 100644\r
 126 --- a/lib/notmuch.h\r
 127 +++ b/lib/notmuch.h\r
 128 @@ -771,6 +771,17 @@ notmuch_message_set_flag (notmuch_message_t *message,\r
 129  time_t\r
 130  notmuch_message_get_date  (notmuch_message_t *message);\r
 131  \r
 132 +/* Set the author member of 'message' - this is the representation used\r
 133 + * when displaying the message\r
 134 + */\r
 135 +void\r
 136 +notmuch_message_set_author (notmuch_message_t *message, const char *author);\r
 137 +\r
 138 +/* Get the author member of 'message'\r
 139 + */\r
 140 +const char *\r
 141 +notmuch_message_get_author (notmuch_message_t *message);\r
 142 +\r
 143  /* Get the value of the specified header from 'message'.\r
 144   *\r
 145   * The value will be read from the actual message file, not from the\r
 146 diff --git a/lib/thread.cc b/lib/thread.cc\r
 147 index 48c070e..c3c83a3 100644\r
 148 --- a/lib/thread.cc\r
 149 +++ b/lib/thread.cc\r
 150 @@ -32,6 +32,7 @@ struct _notmuch_thread {\r
 151      char *subject;\r
 152      GHashTable *authors_hash;\r
 153      char *authors;\r
 154 +    char *nonmatched_authors;\r
 155      GHashTable *tags;\r
 156  \r
 157      notmuch_message_list_t *message_list;\r
 158 @@ -69,8 +70,95 @@ _thread_add_author (notmuch_thread_t *thread,\r
 159         thread->authors = talloc_asprintf (thread, "%s, %s",\r
 160                                            thread->authors,\r
 161                                            author);\r
 162 -    else\r
 163 +    else {\r
 164         thread->authors = talloc_strdup (thread, author);\r
 165 +    }\r
 166 +}\r
 167 +\r
 168 +/*\r
 169 + * move authors of matched messages in the thread to \r
 170 + * the front of the authors list, but keep them in\r
 171 + * oldest first order within their group\r
 172 + */\r
 173 +static void\r
 174 +_thread_move_matched_author (notmuch_thread_t *thread,\r
 175 +                            const char *author)\r
 176 +{\r
 177 +    char *authorscopy;\r
 178 +    char *currentauthor;\r
 179 +    int idx,nmstart,author_len,authors_len;\r
 180 +\r
 181 +    if (thread->authors == NULL)\r
 182 +       return;\r
 183 +    if (thread->nonmatched_authors == NULL)\r
 184 +       thread->nonmatched_authors = thread->authors;\r
 185 +    author_len = strlen(author);\r
 186 +    authors_len = strlen(thread->authors);\r
 187 +    if (author_len == authors_len) {\r
 188 +       /* just one author */\r
 189 +       thread->nonmatched_authors += author_len;\r
 190 +       return;\r
 191 +    }\r
 192 +    currentauthor = strcasestr(thread->authors, author);\r
 193 +    if (currentauthor == NULL)\r
 194 +       return;\r
 195 +    idx = currentauthor - thread->nonmatched_authors;\r
 196 +    if (idx < 0) {\r
 197 +       /* already among matched authors */\r
 198 +       return;\r
 199 +    }\r
 200 +    if (thread->nonmatched_authors + author_len < thread->authors + authors_len) {\r
 201 +       /* we have to make changes, so let's get a temp copy */\r
 202 +       authorscopy = strdup(thread->authors);\r
 203 +       if (authorscopy == NULL)\r
 204 +           return;\r
 205 +       /* nmstart is the offset into where the non-matched authors start */\r
 206 +       nmstart = thread->nonmatched_authors - thread->authors;\r
 207 +       /* copy this author and add the "| " - the if clause above tells us there's more */\r
 208 +       strncpy(thread->nonmatched_authors,author,author_len);\r
 209 +       strncpy(thread->nonmatched_authors+author_len,"| ",2);\r
 210 +       thread->nonmatched_authors += author_len+2;     \r
 211 +       if (idx > 0) {\r
 212 +         /* we are actually moving authors around, not just changing the separator\r
 213 +          * first copy the authors that came BEFORE our author */\r
 214 +         strncpy(thread->nonmatched_authors, authorscopy+nmstart, idx-2);\r
 215 +         /* finally, if there are authors AFTER our author, copy those */\r
 216 +         if(author_len+nmstart+idx < authors_len) {\r
 217 +           strncpy(thread->nonmatched_authors + idx - 2,", ",2);\r
 218 +           strncpy(thread->nonmatched_authors + idx, authorscopy+nmstart + idx + author_len + 2, \r
 219 +                   authors_len - (nmstart + idx + author_len + 2));\r
 220 +         }\r
 221 +       }\r
 222 +       free(authorscopy);\r
 223 +    } else {\r
 224 +       thread->nonmatched_authors += author_len;\r
 225 +    }\r
 226 +    return;\r
 227 +}\r
 228 +\r
 229 +/* clean up the uggly "Lastname, Firstname" format to be "Firstname Lastname" \r
 230 + */\r
 231 +char *\r
 232 +_thread_cleanup_author (notmuch_thread_t *thread,\r
 233 +                       const char *author)\r
 234 +{\r
 235 +  char *cleanauthor;\r
 236 +  const char *comma;\r
 237 +  int fname,lname;\r
 238 +\r
 239 +  cleanauthor = talloc_strdup(thread, author);\r
 240 +  if (cleanauthor == NULL)\r
 241 +    return NULL;\r
 242 +  comma = strchr(author,',');\r
 243 +  if (comma) {\r
 244 +    lname = comma - author;\r
 245 +    fname = strlen(author) - lname - 2;\r
 246 +    strncpy(cleanauthor, comma + 2, fname);\r
 247 +    *(cleanauthor+fname) = ' ';\r
 248 +    strncpy(cleanauthor + fname + 1, author, lname);\r
 249 +    *(cleanauthor+fname+1+lname) = '\0';\r
 250 +  }\r
 251 +  return cleanauthor;\r
 252  }\r
 253  \r
 254  /* Add 'message' as a message that belongs to 'thread'.\r
 255 @@ -87,6 +175,7 @@ _thread_add_message (notmuch_thread_t *thread,\r
 256      InternetAddressList *list;\r
 257      InternetAddress *address;\r
 258      const char *from, *author;\r
 259 +    char *cleanauthor;\r
 260  \r
 261      _notmuch_message_list_add_message (thread->message_list,\r
 262                                        talloc_steal (thread, message));\r
 263 @@ -107,7 +196,9 @@ _thread_add_message (notmuch_thread_t *thread,\r
 264                 mailbox = INTERNET_ADDRESS_MAILBOX (address);\r
 265                 author = internet_address_mailbox_get_addr (mailbox);\r
 266             }\r
 267 -           _thread_add_author (thread, author);\r
 268 +           cleanauthor = _thread_cleanup_author (thread, author);\r
 269 +           _thread_add_author (thread, cleanauthor );\r
 270 +           notmuch_message_set_author (message, cleanauthor);\r
 271         }\r
 272         g_object_unref (G_OBJECT (list));\r
 273      }\r
 274 @@ -150,6 +241,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,\r
 275         notmuch_message_set_flag (hashed_message,\r
 276                                   NOTMUCH_MESSAGE_FLAG_MATCH, 1);\r
 277      }\r
 278 +    _thread_move_matched_author (thread,notmuch_message_get_author(hashed_message));\r
 279  }\r
 280  \r
 281  static void\r
 282 @@ -251,6 +343,7 @@ _notmuch_thread_create (void *ctx,\r
 283      thread->authors_hash = g_hash_table_new_full (g_str_hash, g_str_equal,\r
 284                                                   free, NULL);\r
 285      thread->authors = NULL;\r
 286 +    thread->nonmatched_authors = NULL;\r
 287      thread->tags = g_hash_table_new_full (g_str_hash, g_str_equal,\r
 288                                           free, NULL);\r
 289  \r
 290 \r
 291 \r
 292 -- \r
 293 Dirk Hohndel\r
 294 Intel Open Source Technology Center\r