f9/09bd86b8150b7f137337b0a992995081ce5866

   1 Return-Path: <amdragon@mit.edu>\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 613AC421191\r
   6         for <notmuch@notmuchmail.org>; Sat, 14 Jan 2012 16:05:37 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\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 0S5G8lKSaSED for <notmuch@notmuchmail.org>;\r
  16         Sat, 14 Jan 2012 16:05:36 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-5.mit.edu (DMZ-MAILSEC-SCANNER-5.MIT.EDU\r
  18         [18.7.68.34])\r
  19         by olra.theworths.org (Postfix) with ESMTP id C1149421176\r
  20         for <notmuch@notmuchmail.org>; Sat, 14 Jan 2012 16:05:36 -0800 (PST)\r
  21 X-AuditID: 12074422-b7fd66d0000008f9-8f-4f1218502875\r
  22 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36])\r
  23         by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id E6.53.02297.058121F4; Sat, 14 Jan 2012 19:05:36 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q0F05aJc020421; \r
  27         Sat, 14 Jan 2012 19:05:36 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q0F05YLH026525\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Sat, 14 Jan 2012 19:05:35 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@MIT.EDU>)\r
  36         id 1RmDbH-0002Id-Iv; Sat, 14 Jan 2012 19:05:31 -0500\r
  37 Date: Sat, 14 Jan 2012 19:05:31 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Jameson Graef Rollins <jrollins@finestructure.net>\r
  40 Subject: Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags\r
  41         from queries\r
  42 Message-ID: <20120115000531.GF1801@mit.edu>\r
  43 References: <1326258173-21163-1-git-send-email-amdragon@mit.edu>\r
  44         <1326496024-14403-1-git-send-email-amdragon@mit.edu>\r
  45         <1326496024-14403-3-git-send-email-amdragon@mit.edu>\r
  46         <87zkdpsvln.fsf@servo.finestructure.net>\r
  47 MIME-Version: 1.0\r
  48 Content-Type: text/plain; charset=utf-8\r
  49 Content-Disposition: inline\r
  50 Content-Transfer-Encoding: 8bit\r
  51 In-Reply-To: <87zkdpsvln.fsf@servo.finestructure.net>\r
  52 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  53 X-Brightmail-Tracker:\r
  54  H4sIAAAAAAAAA+NgFlrHKsWRmVeSWpSXmKPExsUixG6nohsgIeRv8PyrpEXTdGeLPfu8LK7f\r
  55         nMnswOxx9zSXx637r9k9nq26xRzAHMVlk5Kak1mWWqRvl8CV8f74G5aCCWIV718cYG1gfCbY\r
  56         xcjJISFgIrF86gt2CFtM4sK99WxdjFwcQgL7GCUmr1zACOFsYJQ48PIFC0iVkMBJJon9t+Mh\r
  57         EkuAEmvXMIIkWARUJTZ1vGICsdkENCS27V8OFhcRMJPo+fIHzGYW0JF4ffg90CAODmGBGIlT\r
  58         b4VAwrwC2hKLp19kh5h5k1Hi5qaZjBAJQYmTM5+wQPSqS/yZd4kZpJdZQFpi+T8OiLC8RPPW\r
  59         2cwgNqeAqcSlp/vZQGxRARWJKSe3sU1gFJ6FZNIsJJNmIUyahWTSAkaWVYyyKblVurmJmTnF\r
  60         qcm6xcmJeXmpRbqmermZJXqpKaWbGMFR4aK0g/HnQaVDjAIcjEo8vIU5Av5CrIllxZW5hxgl\r
  61         OZiURHnXigv5C/El5adUZiQWZ8QXleakFh9ilOBgVhLhXcAKlONNSaysSi3Kh0lJc7AoifOq\r
  62         a73zExJITyxJzU5NLUgtgsnKcHAoSfC+BhkqWJSanlqRlplTgpBm4uAEGc4DNPwZSA1vcUFi\r
  63         bnFmOkT+FKMux8m1V84xCrHk5eelSonz7gIpEgApyijNg5sDS2avGMWB3hLmXQdSxQNMhHCT\r
  64         XgEtYQJaUpbCB7KkJBEhJdXAuGLXL27TEK/Jjt1FUSd+qB5gfPRE96PiejFdoTJO66BFd7hP\r
  65         rWB+yPZj39Me7YT0SGXWDzqmniw/1m0N1I36U/zrlqPIt2m6wkFPmMr637DOS9BxFOVTmuYd\r
  66         0V6iHmy0oHRZc6ztlqSsuB0aj1bqatovsVxpmRgiLhb+8Ostp5Cw5XM1JZiUWIozEg21mIuK\r
  67         EwFJWWkXQQMAAA==\r
  68 Cc: notmuch@notmuchmail.org\r
  69 X-BeenThere: notmuch@notmuchmail.org\r
  70 X-Mailman-Version: 2.1.13\r
  71 Precedence: list\r
  72 List-Id: "Use and development of the notmuch mail system."\r
  73         <notmuch.notmuchmail.org>\r
  74 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  76 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  77 List-Post: <mailto:notmuch@notmuchmail.org>\r
  78 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  79 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  80         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  81 X-List-Received-Date: Sun, 15 Jan 2012 00:05:37 -0000\r
  82 \r
  83 Quoth Jameson Graef Rollins on Jan 14 at  3:38 pm:\r
  84 > It looks like something in this patch is causing the following build\r
  85 > warning:\r
  86 > \r
  87 > CXX -O2 lib/query.o\r
  88 > lib/query.cc:26:8: warning: ‘_notmuch_query’ declared with greater visibility than the type of its field\r
  89 > ‘_notmuch_query::exclude_terms’ [-Wattributes]\r
  90 > \r
  91 > However, I can't quite figure out what's causing it.\r
  92 \r
  93 The problem is that notmuch_query_t is a "visible" symbol because the\r
  94 type is declared (though not defined) in lib/notmuch.h.  The actual\r
  95 definition is tucked away in lib/query.cc, but GCC doesn't seem to\r
  96 care.  I added a field of type notmuch_string_list_t to the struct's\r
  97 definition, but notmuch_string_list_t is declared between the "hidden"\r
  98 pragmas in lib/notmuch-private.h because the type is private to the\r
  99 library.  This field with a hidden type in a visible type is what GCC\r
 100 is complaining about.\r
 101 \r
 102 I'm rather confused by the whole type visibility thing since type\r
 103 symbols aren't linkable anyway.  However, while puzzling over how you\r
 104 could possibly use hidden types if they can only be used in other\r
 105 hidden types, I discovered that Carl had solved this exact problem\r
 106 last May in d5523ead by adding a visibility("default") attribute to\r
 107 the offending hidden type.\r
 108 \r
 109 > > +void\r
 110 > > +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)\r
 111 > > +{\r
 112 > > +    char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);\r
 113 > > +    _notmuch_string_list_append (query->exclude_terms, term);\r
 114 > > +}\r
 115 > \r
 116 > This is really not an issue with this patch at all, and it should NOT\r
 117 > prevent it from being applied, but this came up briefly on IRC and I'm\r
 118 > curious, so I'll ask about it here.\r
 119 > \r
 120 > Are terms ALWAYS lower cased?  If not, it seems to me it's possible to\r
 121 > have an indexed term 'Kspam' that would get confused with the term\r
 122 > 'spam' prefixed with the keyword prefix 'K' (which we use for tags).\r
 123 > Maybe this degeneracy is broken by the query parser somehow (or maybe by\r
 124 > the fact that tags are boolean terms?), but I wonder if it's not safer\r
 125 > to use the built-in xapian prefix separator ':', ie:\r
 126 > \r
 127 >   ... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);\r
 128 > \r
 129 > I guess fixing that globally would require a database rebuild...\r
 130 \r
 131 We discussed this on IRC, but to summarize for the list, the tag\r
 132 prefix is a single character, so Xapian's ':' rule doesn't apply.\r
 133 There are several places where we *do* get this wrong and use a\r
 134 multi-character term prefix with a term that may start with a capital\r
 135 letter but they're all terms you can't search anyway and, unless I'm\r
 136 mistaken, we're completely consistent about where we violate or do not\r
 137 violate the ':' rule.\r
 138 \r
 139 > Ok, that's totally just an aside, and should not be a blocker for this\r
 140 > patch.\r
 141 > \r
 142 > jamie.\r