From: Mark Walters <markwalters1009@gmail.com>
To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/6] cli: sanitize tabs to spaces in notmuch search
In-Reply-To:
 <af8381b86275c4be77cc8527a19d69efbd2e9423.1381948853.git.jani@nikula.org>
References: <cover.1381948853.git.jani@nikula.org>
	<af8381b86275c4be77cc8527a19d69efbd2e9423.1381948853.git.jani@nikula.org>
User-Agent: Notmuch/0.15.2+334~gafd5c6a (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Thu, 17 Oct 2013 09:07:46 +0100
Message-ID: <87ob6ob9rh.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


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!

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.

Also why only tabs to spaces but \n to a '?'

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