Re: [RFC/PATCH] Add search-files command

[Ali Polatel <alip@exherbo.org>]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

Fernando Carrijo yazmış:
> Hi Ali,

Hey Fernando,

> On Wed, 13 Jan 2010 12:24:22 +0200, Ali Polatel <alip@exherbo.org> wrote:
<snip>
> > diff --git a/notmuch-search-files.c b/notmuch-search-files.c
> > new file mode 100644
> > index 0000000..b48783a
> > --- /dev/null
> > +++ b/notmuch-search-files.c
> > @@ -0,0 +1,107 @@
> > +/* notmuch - Not much of an email program, (just index and search)
> > + *
> > + * Copyright © 2009 Carl Worth
> 
> I almost certainly should not be speaking in the name of Carl, but a
> few weeks ago he replied with the following words to a message which
> somehow raised the discussion of copyright holding in notmuch source
> code:
> 
>   "Please feel free to retain your own copyright. I certainly don't
>    deserve anything being assigned to me."
> 
> The message-id of his reply is: 873a34tf8b.fsf@yoom.home.cworth.org,
> just in the case you want to check the context in which it occurred.
> 

Thanks, I don't really care about who owns the copyright as long as it's
open source but I'll change it if that's what Carl wishes.

<snip>
> > +    if (*query_str == '\0') {
> > +	fprintf (stderr, "Error: notmuch search-files requires at least one search term.\n");
> 
> Could we break this line so that it did not go beyond the column limit
> set by the coding conventions? Maybe we should go one step further and
> import the file CODING_STYLE from cairo into notmuch's repository.
> 

Sure, I didn't know much about the coding style and as you said there's
no document about it in notmuch's repository.

> > +	return 1;
> > +    }
> > +
> > +    query = notmuch_query_create (notmuch, query_str);
> > +    if (query == NULL) {
> > +	fprintf (stderr, "Out of memory\n");
> > +	return 1;
> > +    }
> > +
> > +    notmuch_query_set_sort (query, sort);
> > +
> > +    do_search_files (query);
> > +
> > +    notmuch_query_destroy (query);
> > +    notmuch_database_close (notmuch);
> 
> It is not something of major importance, but I realized that the
> function notmuch_config_close is only being called when notmuch is
> executed with no arguments. As we know, the kernel shall close all
> open file descriptors upon program termination, but wouldn't it be
> better if we did it explicitly? Or did I overlook something?
> 

Nope, it's good coding practise to free all resources before exit, I
didn't do it for config because neither the search command does it.
I'll send an updated patch soonish and I'll also send a separate patch
for notmuch-search.c as well.

> Kind regards,
> Fernando Carrijo.
> 

-- 
Regards,
Ali Polatel

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]