bf/4fae68f28013c4228f3d713378d572557d4b8f

   1 Return-Path: <sojkam1@fel.cvut.cz>\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 6140E4196F0\r
   6         for <notmuch@notmuchmail.org>; Fri,  9 Apr 2010 18:54:08 -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: -1.9\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5\r
  12         tests=[BAYES_00=-1.9] 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 y-ZgeBoOYSjB for <notmuch@notmuchmail.org>;\r
  16         Fri,  9 Apr 2010 18:54:07 -0700 (PDT)\r
  17 Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 1B766431FC1\r
  19         for <notmuch@notmuchmail.org>; Fri,  9 Apr 2010 18:54:07 -0700 (PDT)\r
  20 Received: from localhost (unknown [192.168.200.4])\r
  21         by max.feld.cvut.cz (Postfix) with ESMTP id 0C39D19F341A;\r
  22         Sat, 10 Apr 2010 03:54:06 +0200 (CEST)\r
  23 X-Virus-Scanned: IMAP AMAVIS\r
  24 Received: from max.feld.cvut.cz ([192.168.200.1])\r
  25         by localhost (styx.feld.cvut.cz [192.168.200.4]) (amavisd-new,\r
  26         port 10044)\r
  27         with ESMTP id PkPPgZssXM8R; Sat, 10 Apr 2010 03:54:04 +0200 (CEST)\r
  28 Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34])\r
  29         by max.feld.cvut.cz (Postfix) with ESMTP id A03D519F3411;\r
  30         Sat, 10 Apr 2010 03:54:04 +0200 (CEST)\r
  31 Received: from wsheee.localdomain (unknown [213.29.198.144])\r
  32         (Authenticated sender: sojkam1)\r
  33         by imap.feld.cvut.cz (Postfix) with ESMTPSA id 2BCACFA003;\r
  34         Sat, 10 Apr 2010 03:54:00 +0200 (CEST)\r
  35 Received: from wsh by wsheee.localdomain with local (Exim 4.69)\r
  36         (envelope-from <sojkam1@fel.cvut.cz>)\r
  37         id 1O0PtX-0001UM-PY; Sat, 10 Apr 2010 03:53:59 +0200\r
  38 From: Michal Sojka <sojkam1@fel.cvut.cz>\r
  39 To: Dirk Hohndel <hohndel@infradead.org>, notmuch <notmuch@notmuchmail.org>\r
  40 Subject: Re: [RFC] reordering and cleanup of thread authors\r
  41 In-Reply-To: <m3zl1cfsb4.fsf@x200.gr8dns.org>\r
  42 References: <m31veru7vn.fsf@x200.gr8dns.org> <87zl1d5fc0.fsf@steelpick.2x.cz>\r
  43         <m3zl1cfsb4.fsf@x200.gr8dns.org>\r
  44 Date: Sat, 10 Apr 2010 03:53:59 +0200\r
  45 Message-ID: <87aatcysw8.fsf@wsheee.localdomain>\r
  46 MIME-Version: 1.0\r
  47 Content-Type: text/plain; charset=us-ascii\r
  48 X-BeenThere: notmuch@notmuchmail.org\r
  49 X-Mailman-Version: 2.1.13\r
  50 Precedence: list\r
  51 List-Id: "Use and development of the notmuch mail system."\r
  52         <notmuch.notmuchmail.org>\r
  53 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  54         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  55 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  56 List-Post: <mailto:notmuch@notmuchmail.org>\r
  57 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  58 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  59         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  60 X-List-Received-Date: Sat, 10 Apr 2010 01:54:08 -0000\r
  61 \r
  62 On Fri, 09 Apr 2010, Dirk Hohndel wrote:\r
  63 > On Fri, 09 Apr 2010 08:07:27 +0200, Michal Sojka <sojkam1@fel.cvut.cz> wrote:\r
  64 > > On Wed, 07 Apr 2010, Dirk Hohndel wrote:\r
  65 > > > \r
  66 > > > This is based in part on some discussion on IRC today.\r
  67 > > > When a thread is displayed in the search results, previously the authors\r
  68 > > > were always displayed in chronological order. But if only part of the\r
  69 > > > thread matches the query, that may or may not be the intuitive thing to\r
  70 > > > do.\r
  71 > > \r
  72 > > thanks for the patch. It is a very interesting feature.\r
  73 > \r
  74 > Thanks - I've been using it for a few days now and am fairly happy with\r
  75 > it.\r
  76 > \r
  77 > > >\r
  78 > > > +/*\r
  79 > > > + * move authors of matched messages in the thread to \r
  80 > > > + * the front of the authors list, but keep them in\r
  81 > > > + * oldest first order within their group\r
  82 > > > + */\r
  83 > > > +static void\r
  84 > > > +_thread_move_matched_author (notmuch_thread_t *thread,\r
  85 > > > +                      const char *author)\r
  86 > > > +{\r
  87 > > > +    char *authorscopy;\r
  88 > > > +    char *currentauthor;\r
  89 > > > +    int idx,nmstart,author_len,authors_len;\r
  90 > > > +\r
  91 > > > +    if (thread->authors == NULL)\r
  92 > > > + return;\r
  93 > > > +    if (thread->nonmatched_authors == NULL)\r
  94 > > > + thread->nonmatched_authors = thread->authors;\r
  95 > > > +    author_len = strlen(author);\r
  96 > > > +    authors_len = strlen(thread->authors);\r
  97 > > > +    if (author_len == authors_len) {\r
  98 > > > + /* just one author */\r
  99 > > > + thread->nonmatched_authors += author_len;\r
 100 > > > + return;\r
 101 > > > +    }\r
 102 > > > +    currentauthor = strcasestr(thread->authors, author);\r
 103 > > > +    if (currentauthor == NULL)\r
 104 > > > + return;\r
 105 > > > +    idx = currentauthor - thread->nonmatched_authors;\r
 106 > > > +    if (idx < 0) {\r
 107 > > > + /* already among matched authors */\r
 108 > > > + return;\r
 109 > > > +    }\r
 110 > > > +    if (thread->nonmatched_authors + author_len < thread->authors + authors_len) {\r
 111 > > \r
 112 > > What does the above condition exactly mean? \r
 113 > \r
 114 > Eh - it's ugly. \r
 115 > \r
 116 > thread->authors + authors_len points to the trailing '\0' of the list of\r
 117 > all my authors. At this point in the code we know that the current\r
 118 > position of this author is at or after nonmatched_authors. So if\r
 119 > nonmatched_authors + author_len is less than the end of the all authors\r
 120 > that means that there was another author in the list behind this one -\r
 121 > and we need to move things around. \r
 122 > \r
 123 > In the else clause we just move the pointer to nonmatched_authors to the\r
 124 > end of this author.\r
 125 > \r
 126 > So yeah, ugly, should be rewritten to make it easier to understand (or\r
 127 > at least commented better).\r
 128 > \r
 129 > > I was not able to decode it\r
 130 > > and it seems to be wrong. I expect that the "|" is used to delimit\r
 131 > > matched and non-matched authors. If I run notmuch search\r
 132 > > thread:XXXXXXXXXXXXXXXX, which matches all messages in the thread, I see\r
 133 > > all authors delimited by "|", which I consider wrong.\r
 134 > \r
 135 > Why do you think it's wrong? \r
 136 \r
 137 Because I thought, that | was used as a delimiter between the two parts\r
 138 of the list and not as a marker of matched authors.\r
 139 \r
 140 > It's consistent. The thing that I actually DISlike about the current\r
 141 > solution is that the last author has no delimiter (since there's no\r
 142 > trailing ',' in the list and I didn't want to realloc the space for\r
 143 > the authors string). So you can't tell with one glance if all authors\r
 144 > match or all but the last one match. I haven't come up with a better\r
 145 > visual way to indicate this...\r
 146 \r
 147 I think that using | as a separator would help here. Let's say that\r
 148 initially we have "Matched Author, Non Matched, Matched Again" we can\r
 149 tranform this to  "Matched Author, Matched Again| Non Matched". This way,\r
 150 the length of the string remains the same. If there is no | after\r
 151 transformation, we know that all authors matched because there is always\r
 152 at least one mathed author in the search results.\r
 153 \r
 154 -Michal\r
 155 \r