ee/a8c989a200e931e6f3ca8eee0d907872de3aa6

   1 Return-Path: <BATV+95f6b7a4aaa7c13215e8+2420+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 503894196F0\r
   6         for <notmuch@notmuchmail.org>; Fri,  9 Apr 2010 10:29:23 -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 Fjtub7hVIGXN for <notmuch@notmuchmail.org>;\r
  16         Fri,  9 Apr 2010 10:29:22 -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 42708431FC1\r
  19         for <notmuch@notmuchmail.org>; Fri,  9 Apr 2010 10:29:22 -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 1O0I1A-0005Mv-5Y; Fri, 09 Apr 2010 17:29:20 +0000\r
  23 Received: by localhost.localdomain (Postfix, from userid 500)\r
  24         id 986C2C0052; Fri,  9 Apr 2010 10:29:19 -0700 (PDT)\r
  25 From: Dirk Hohndel <hohndel@infradead.org>\r
  26 To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch <notmuch@notmuchmail.org>\r
  27 Subject: Re: [RFC] reordering and cleanup of thread authors\r
  28 In-Reply-To: <87zl1d5fc0.fsf@steelpick.2x.cz>\r
  29 References: <m31veru7vn.fsf@x200.gr8dns.org> <87zl1d5fc0.fsf@steelpick.2x.cz>\r
  30 Date: Fri, 09 Apr 2010 10:29:19 -0700\r
  31 Message-ID: <m3zl1cfsb4.fsf@x200.gr8dns.org>\r
  32 MIME-Version: 1.0\r
  33 Content-Type: text/plain; charset=us-ascii\r
  34 X-SRS-Rewrite: SMTP reverse-path rewritten from <hohndel@infradead.org> by\r
  35         bombadil.infradead.org See http://www.infradead.org/rpr.html\r
  36 X-BeenThere: notmuch@notmuchmail.org\r
  37 X-Mailman-Version: 2.1.13\r
  38 Precedence: list\r
  39 List-Id: "Use and development of the notmuch mail system."\r
  40         <notmuch.notmuchmail.org>\r
  41 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  42         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  43 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  44 List-Post: <mailto:notmuch@notmuchmail.org>\r
  45 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  46 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  47         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  48 X-List-Received-Date: Fri, 09 Apr 2010 17:29:23 -0000\r
  49 \r
  50 On Fri, 09 Apr 2010 08:07:27 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:\r
  51 > On Wed, 07 Apr 2010, Dirk Hohndel wrote:\r
  52 > > \r
  53 > > This is based in part on some discussion on IRC today.\r
  54 > > When a thread is displayed in the search results, previously the authors\r
  55 > > were always displayed in chronological order. But if only part of the\r
  56 > > thread matches the query, that may or may not be the intuitive thing to\r
  57 > > do.\r
  58 > \r
  59 > thanks for the patch. It is a very interesting feature.\r
  60 \r
  61 Thanks - I've been using it for a few days now and am fairly happy with\r
  62 it.\r
  63 \r
  64 > >\r
  65 > > +/*\r
  66 > > + * move authors of matched messages in the thread to \r
  67 > > + * the front of the authors list, but keep them in\r
  68 > > + * oldest first order within their group\r
  69 > > + */\r
  70 > > +static void\r
  71 > > +_thread_move_matched_author (notmuch_thread_t *thread,\r
  72 > > +                        const char *author)\r
  73 > > +{\r
  74 > > +    char *authorscopy;\r
  75 > > +    char *currentauthor;\r
  76 > > +    int idx,nmstart,author_len,authors_len;\r
  77 > > +\r
  78 > > +    if (thread->authors == NULL)\r
  79 > > +   return;\r
  80 > > +    if (thread->nonmatched_authors == NULL)\r
  81 > > +   thread->nonmatched_authors = thread->authors;\r
  82 > > +    author_len = strlen(author);\r
  83 > > +    authors_len = strlen(thread->authors);\r
  84 > > +    if (author_len == authors_len) {\r
  85 > > +   /* just one author */\r
  86 > > +   thread->nonmatched_authors += author_len;\r
  87 > > +   return;\r
  88 > > +    }\r
  89 > > +    currentauthor = strcasestr(thread->authors, author);\r
  90 > > +    if (currentauthor == NULL)\r
  91 > > +   return;\r
  92 > > +    idx = currentauthor - thread->nonmatched_authors;\r
  93 > > +    if (idx < 0) {\r
  94 > > +   /* already among matched authors */\r
  95 > > +   return;\r
  96 > > +    }\r
  97 > > +    if (thread->nonmatched_authors + author_len < thread->authors + authors_len) {\r
  98 > \r
  99 > What does the above condition exactly mean? \r
 100 \r
 101 Eh - it's ugly. \r
 102 \r
 103 thread->authors + authors_len points to the trailing '\0' of the list of\r
 104 all my authors. At this point in the code we know that the current\r
 105 position of this author is at or after nonmatched_authors. So if\r
 106 nonmatched_authors + author_len is less than the end of the all authors\r
 107 that means that there was another author in the list behind this one -\r
 108 and we need to move things around. \r
 109 \r
 110 In the else clause we just move the pointer to nonmatched_authors to the\r
 111 end of this author.\r
 112 \r
 113 So yeah, ugly, should be rewritten to make it easier to understand (or\r
 114 at least commented better).\r
 115 \r
 116 > I was not able to decode it\r
 117 > and it seems to be wrong. I expect that the "|" is used to delimit\r
 118 > matched and non-matched authors. If I run notmuch search\r
 119 > thread:XXXXXXXXXXXXXXXX, which matches all messages in the thread, I see\r
 120 > all authors delimited by "|", which I consider wrong.\r
 121 \r
 122 Why do you think it's wrong? It's consistent. The thing that I actually\r
 123 DISlike about the current solution is that the last author has no\r
 124 delimiter (since there's no trailing ',' in the list and I didn't want\r
 125 to realloc the space for the authors string). So you can't tell with one\r
 126 glance if all authors match or all but the last one match. I haven't\r
 127 come up with a better visual way to indicate this... \r
 128 \r
 129 Suggestions?\r
 130 \r
 131 /D\r
 132 \r
 133 -- \r
 134 Dirk Hohndel\r
 135 Intel Open Source Technology Center\r