From: Austin Clements <aclements@csail.mit.edu>
To: David Bremner <david@tethera.net>
Subject: Re: [PATCH 06/11] lib: Internal support for querying and creating
	ghost	messages
In-Reply-To: <87y4stoh41.fsf@maritornes.cs.unb.ca>
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>
	<87y4stoh41.fsf@maritornes.cs.unb.ca>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Mon, 06 Oct 2014 12:03:56 -0400
Message-ID: <87k34dchhv.fsf@csail.mit.edu>
MIME-Version: 1.0
Content-Type: text/plain
Cc: notmuch@notmuchmail.org
Precedence: list

On Mon, 06 Oct 2014, David Bremner <david@tethera.net> wrote:
> 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.

I added some macros for doing this to notmuch-private.h and converted
the other bit twiddling for message flags to use these.

>>> > +	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.

A #define by BOOLEAN_PREFIX_INTERNAL won't help because
BOOLEAN_PREFIX_INTERNAL lives in database.cc and this code is in
message.cc.  I would have to put the #define in one of the private
headers, but I could use it in BOOLEAN_PREFIX_INTERNAL so there wouldn't
be any duplication of the "T" string.

I added a comment pointing to BOOLEAN_PREFIX_INTERNAL.  Maybe that's
enough?

I'll post v2 later today, when I can run the test suite (currently
running on battery).