From: Jani Nikula <jani@nikula.org>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/6] cli: sanitize tabs to spaces in notmuch search
In-Reply-To: <87ob6ob9rh.fsf@qmul.ac.uk>
References: <cover.1381948853.git.jani@nikula.org>
	<af8381b86275c4be77cc8527a19d69efbd2e9423.1381948853.git.jani@nikula.org>
	<87ob6ob9rh.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.16+97~g6878b0b (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 17 Oct 2013 14:47:18 +0300
Message-ID: <87k3hcqfuh.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Thu, 17 Oct 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> Hi
>
> I have looked at the whole series and broadly it looks good. However, I
> don't know this code so this is not a full review. I do have a few
> comments: some of these may be plain wrong in which case my apologies!

Thanks for the review!

> On Wed, 16 Oct 2013, Jani Nikula <jani@nikula.org> wrote:
>
>> This is in preparation of switching to gmime header parsing, but
>> arguably converting tabs to spaces rather than question marks is the
>> right thing to do anyway.
>> ---
>
> I think it would be worth saying in the commit message that this is only
> for text summary output.

Agreed.

> Also why only tabs to spaces but \n to a '?'

The notmuch header parser converts the tabs in header folding to spaces,
gmime keeps them as tabs. AFAICT that is the only difference between
headers indexed by notmuch and gmime, and causes tests to fail. So we do
the tabs to spaces conversion here to produce same output from cli. This
patch just does the minimal change required for tests to pass; other
changes might be good too, but they were not relevant for this series.

BR,
Jani.

>
> Best wishes
>
> Mark
>
>
>
>
>>  notmuch-search.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index d9d39ec..eab314f 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -40,7 +40,9 @@ sanitize_string (const void *ctx, const char *str)
>>      loop = out = talloc_strdup (ctx, str);
>>  
>>      for (; *loop; loop++) {
>> -	if ((unsigned char)(*loop) < 32)
>> +	if (*loop == '\t')
>> +	    *loop = ' ';
>> +	else if ((unsigned char)(*loop) < 32)
>>  	    *loop = '?';
>>      }
>>      return out;
>> -- 
>> 1.8.4.rc3
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch