From: Jameson Graef Rollins <jrollins@finestructure.net>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags
	from queries
In-Reply-To: <1326496024-14403-3-git-send-email-amdragon@mit.edu>
References: <1326258173-21163-1-git-send-email-amdragon@mit.edu>
	<1326496024-14403-1-git-send-email-amdragon@mit.edu>
	<1326496024-14403-3-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.11+63~g6965a07 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Sat, 14 Jan 2012 15:38:44 -0800
Message-ID: <87zkdpsvln.fsf@servo.finestructure.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

It looks like something in this patch is causing the following build
warning:

CXX -O2 lib/query.o
lib/query.cc:26:8: warning: =E2=80=98_notmuch_query=E2=80=99 declared with =
greater visibility than the type of its field
=E2=80=98_notmuch_query::exclude_terms=E2=80=99 [-Wattributes]

However, I can't quite figure out what's causing it.

> +void
> +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
> +{
> +    char *term =3D talloc_asprintf (query, "%s%s", _find_prefix ("tag"),=
 tag);
> +    _notmuch_string_list_append (query->exclude_terms, term);
> +}

This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.

Are terms ALWAYS lower cased?  If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:

  ... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);

I guess fixing that globally would require a database rebuild...

Ok, that's totally just an aside, and should not be a blocker for this
patch.

jamie.

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)

iQIcBAEBCAAGBQJPEhIEAAoJEO00zqvie6q8484QAJvTEE7JhH11U16e5kYiVhj7
/o5gBPgFJeCmadtASS62wcPv8+Cp9hGSnj8zl7vvoTNPiZ0f2vFvineff1n7YJlq
14GXALmUqef+lMVrweKDKlIngZFVpe1leIH6lt63fXEaKOKvf31dH6XAo/hP/lgy
b37qCxwgewouVlbChLBI5WNMmw6syrzEFa/+j0X+3RH33qRs6Jk5V54V77O8b32Y
GvVUkNgSENSl2TEEKVZlAmeYBohjneVftVXQrLCb9+sVWegZB1q65upRm4JXjEHG
B0xzCBG/o6xgoOJyWRXMbnG6YpcAkblaoKqWm/pe9t4GYa4KNlagMwbA+9mtROfO
UbxtDNUZpdbI3xxNKkPumzrEUtRl/7hpcey4JLqd08oekG2WoezzE87F2DQ5mPSZ
atFBncwl6jTUxYmtPRdqBXz3csFyteJpwFZ6xg0+Zkuxg8TRCF2meQZ5Bw45IVlh
+Wjs5f+0t0W3t9mN53XHwiRTvkMaBie0neZZ18b/DTLpMT47cpk393VFt78S+Vhu
jyFbpx9L2uaaidqEeW0zCDQ1gK9pdWl4OCm9inRRzN1hKGN2mKRa+26OUGbWSJ3s
hjzdHWwU8Dl/LqFz9NxTqKX9dOQqXtbR0AfK3KUnorse4kM8B3a4QhsDkd1vntrv
rp21084pVi/TuEgrK8XG
=RbYn
-----END PGP SIGNATURE-----
--=-=-=--