From: David Bremner <david@tethera.net>
To: Austin Clements <aclements@csail.mit.edu>
Subject: Re: [PATCH 06/11] lib: Internal support for querying and creating
	ghost	messages
In-Reply-To: <20141005232400.GC7970@csail.mit.edu>
References: <1412345958-8278-1-git-send-email-aclements@csail.mit.edu>
	<1412345958-8278-7-git-send-email-aclements@csail.mit.edu>
	<87fvf2q5p5.fsf@maritornes.cs.unb.ca>
	<20141005232400.GC7970@csail.mit.edu>
User-Agent: Notmuch/0.18.1+98~gae27403 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Mon, 06 Oct 2014 08:19:10 +0200
Message-ID: <87y4stoh41.fsf@maritornes.cs.unb.ca>
MIME-Version: 1.0
Content-Type: text/plain
Cc: notmuch@notmuchmail.org
Precedence: list

Austin Clements <aclements@csail.mit.edu> writes:

>
> I'm used to reading this stuff, so either way is fine with me.  Do we
> have bit set / clear / read macros?
>

I guess not. the things we have in query.cc are related but different.

>> > +	else if (*i == "Tghost")
>> > +	    message->flags |= (1 << NOTMUCH_MESSAGE_FLAG_GHOST);
>> > +	else
>> 
>> It makes me faintly unhappy to have the prefix hardcoded here.
>> Not sure if there is a sensible solution.
>
> I agree, but I also don't want to construct the test string every time
> or deconstruct the term string every time.  I could move the "T"
> prefix string to a #define and use that both here and in
> BOOLEAN_PREFIX_INTERNAL, but that solution may be worse than the
> problem.  What do you think?

Maybe just a comment to point to BOOLEAN_PREFIX_INTERNAL.

Or maybe define a macro right beside BOOLEAN_PREFIX_INTERNAL like

#define ADD_TYPE_PREFIX(s) "T" s

At least then the duplication is all in one place.