[PATCH] Release memory allocated by internet_address_list_parse_string()
On Wed, 07 Dec 2011 22:13:19 +0200, Tomi Ollila wrote: > g_object_unref() releases the memory of the InternetAddressList object > returned by internet_address_list_parse_string() -- when last (only) > reference is released, internet_address_list_finalize() will do cleanup. > --- > When reviewing, see also gmime-2.4.25/gmime/internet-address.c (or older) > I tested this patch by entering: notmuch show --format=mbox '*' | wc > > Note that this fixes one potential memory leak only in case --format=mbox > as _extract_email_address is called only there. There is at least one bug here -- I failed to check whether addresses is non-NULL before attempting to g_object_unref (). So please ignore that when reviewing whether g_object_unref() is the correct action here. I provide a new patch when I know whether re-check or separate the NULL test in the beginning of _extract_email_address(). > notmuch-show.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/notmuch-show.c b/notmuch-show.c > index 603992a..3abfa07 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -255,7 +255,7 @@ _extract_email_address (const void *ctx, const char *from) > email = talloc_strdup (ctx, email); > >DONE: > -/* XXX: How to free addresses here? */ > +g_object_unref (addresses); > return email; > } > > -- > 1.7.7.3 Tomi
[PATCH] Release memory allocated by internet_address_list_parse_string()
g_object_unref() releases the memory of the InternetAddressList object returned by internet_address_list_parse_string() -- when last (only) reference is released, internet_address_list_finalize() will do cleanup. --- When reviewing, see also gmime-2.4.25/gmime/internet-address.c (or older) I tested this patch by entering: notmuch show --format=mbox '*' | wc Note that this fixes one potential memory leak only in case --format=mbox as _extract_email_address is called only there. notmuch-show.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index 603992a..3abfa07 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -255,7 +255,7 @@ _extract_email_address (const void *ctx, const char *from) email = talloc_strdup (ctx, email); DONE: -/* XXX: How to free addresses here? */ +g_object_unref (addresses); return email; } -- 1.7.7.3
[PATCH] debian: add a recommends for w3m-el or w3m-el-snapshot to notmuch-emacs
From: David Bremner Installing w3m-el provides a nicer out of the box experience for viewing html in notmuch, and the overhead is not too bad (about 5M diskspace). --- debian/control |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/debian/control b/debian/control index f6f415e..369ef07 100644 --- a/debian/control +++ b/debian/control @@ -81,6 +81,7 @@ Breaks: notmuch (<<0.6~254~) Replaces: notmuch (<<0.6~254~) Depends: ${misc:Depends}, notmuch (>= ${source:Version}), emacs23 (>= 23~) | emacs23-nox (>=23~) | emacs23-lucid (>=23~) +Recommends: w3m-el | w3m-snapshot Description: thread-based email index, search and tagging (emacs interface) Notmuch is a system for indexing, searching, reading, and tagging large collections of email messages in maildir or mh format. It uses -- 1.7.7.3
S/MIME support in notmuch
I'd like to report some success on getting S/MIME signature verification working using notmuch and the recently-released GMime 2.6. I specifically tested with notmuch-0.10.2 and gmime-2.6.1. The following changes were required: 1) notmuch: Apply patch from Redhat packaging to handle API changes from gmime-2.4 to gmime-2.6 (see "compile error of current git on F15" thread from 25 November on the list) 2) notmuch: Create a S/MIME context instead of the GPG context in notmuch-show.c. g_mime_gpg_context_new() becomes g_mime_pkcs7_context_new(), and similarly for g_mime_gpg_context_set_always_trust(). 3) gmime: The pkcs7 context only works with signatures of "application/pkcs7-signature". Per RFC2311 section C, both "application/pkcs7-signature" and "application/x-pkcs7-signature" should be treated identically. I temporarily disabled this check in gmime/gmime-multipart-signed.c and then gmime accepted the signatures. Next, I was always seeing signature verification errors with completely unhelpful error messages. These turned out to be because the 'gpg-agent' program was not running. Once I started the agent, I got prompts on trusting root certs and was then able to see known-valid certificates verified in the emacs UI. NB: I started gpg-agent with the --allow-mark-trusted option so that it would graphically prompt me for which root certificates to trust. See http://lists.gnupg.org/pipermail/gnupg-users/2004-September/023247.html for more detail on some of the general setup choices for the GPG S/MIME stack. The most useful command for debugging the underlying S/MIME configuration was "gpgsm --list-chain --with-validation". I don't have submittable patches for #2/#3 yet, but I wanted to share what I found about the scope of what actually needs to be done, which is fairly small. (The biggest blocker is probably that Debian & other distros haven't packaged gmime-2.6.) Dan
Patch Amnesty Day: July 1, 2011
Hi All; As some of you know I have been making some effort to get a handle on the patches to notmuch floating around using nmbug [1]. We are doing pretty well keeping track of current patches, and I started working my way back through things that looked like patches. Sometime around the release of 0.6, this became less rewarding as most patches are either applied or stale. So I decided to draw the line there. If you are the author (or just a user) of some patches that are older than that, you should absolutely feel free to bring them to my/our attention, either by tagging them as notmuch::patch in nmbug, or by following up to the thread on the list. d [1]: http://notmuchmail.org/nmbug/ -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 315 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/594660de/attachment.pgp>
[PATCH v3 0/2] notmuch hooks
On Tue, 06 Dec 2011 18:27:27 -0800, Jameson Graef Rollins wrote: > On Tue, 6 Dec 2011 15:22:36 +0200, Jani Nikula wrote: > > Hi all, this is v3 of the notmuch hooks patches. I think this is nearing > > completion apart from final review and, most notably, tests. > > Hey, Jani. Thanks so much for these patches. I think this is a really > neat idea and it will be very useful. I already know that this will cut > out a lot of extraneous shell scripting that I'm relying on now. Thanks for your interest! > > I've been using this for some days now, and (subjective as it is) I have to > > say > > I like offlineimap being run from notmuch new "pre-new" hook much better > > than > > vice versa. Even more so for "post-new" tagging scripts. > > I think the only thing that I was not particularly clear about is that I > guess the "pre-new" and "post-new" scripts actually need to be scripts > named "pre-new" and "post-new" in the hooks directory. For some reason > I didn't quite grok that from the help pages, and was confused if I > needed to make a new "pre-new" subdirectory in the hooks directory or > what. I think just expecting a script at hook/pre-new is fine, but it > maybe could be made a little more explicit in the documentation. It's of course difficult for me to see this from someone else's point of view, but IMHO all the relevant details can be found in the man page and help. > In fact, I think the best thing would actually be for notmuch new to > make the .notmuch/hooks directory itself and to pre-fill it with "turned > off" hook scripts that contain some useful comments on how to use them. > This is what git does, and I think it's very handy, since it's all > fairly self documenting. The notmuch help could then just refer to the > fact that they're there, and point people to the comments in the various > scripts for how to use them. This is a good idea, and complimentary to the documentation. I'll add this. (I'll just have to come up with good samples. :o) I wonder, though, whether the samples should be created only when creating the database, or whenever the hooks directory does not exist? The latter, I guess. Otherwise existing notmuch users would hardly get the sample hooks. BR, Jani.
[PATCH 1/2] test: date_relative in notmuch search json output
On Sat, 23 Jul 2011 10:54:03 +0100, pazz wrote: > expect the date_relative field for thread entries > in notmuch search's json output > --- I pushed an amended version of this one (explaining why the date won't change in the future), and the next patch as well. d
[PATCH 1/2] emacs: remove some code duplication in notmuch-show
On Sat, 26 Nov 2011 02:23:30 +0400, Dmitry Kurochkin wrote: > Add optional props argument to `notmuch-show-get-header'. Use it to > get headers in `notmuch-show-insert-part-multipart/signed' and > `notmuch-show-insert-part-multipart/encrypted'. Pushed this series. d
[PATCH v4 0/3] emacs: do not call notmuch show for non-inlinable parts
On Wed, 30 Nov 2011 01:19:51 +0400, Dmitry Kurochkin wrote: > Changes: > > v4 since v3: > > * Use $(()) instead of expr(1) for math. > pushed d
New test for exported symbols
On Mon, 28 Nov 2011 22:51:14 -0800, David Bremner wrote: > As long promised, here is a test to compare the list of symbols > retrieved with objdump to those from notmuch.sym > > I didn't verify whether this runs properly during out of tree builds. aidecoe did verify this for me on Gentoo, so I pushed these
[PATCH 2/2] python: annotate all calls into libnotmuch with types
in libnotmuch and turned into the appropriate error code (hm, there is only the generic XAPIAN_EXCEPTION, I thought there was a way to indicate that the db has been modified?). Justus -- next part -- A non-text attachment was scrubbed... Name: .signature Type: application/octet-stream Size: 17 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/37160f77/attachment.obj>
[PATCH v3 0/2] notmuch hooks
On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince wrote: > On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins finestructure.net> wrote: > > Also, what if we make it so that the post-new hook script only runs if > > notmuch new processes new messages? All of my post-new functions don't > > need to be run at all if there is no new mail. I think the post-new hook should be run always (provided there have been no errors). I think it might be surprising not to, and some users might use the hook for something other than tagging. > Or would it make sense to pass this information to the hook somehow? It would, but as I wrote in id:"87mxb8kt5r.fsf at nikula.org", I think that should come as another patch afterwards. I know I can't decide yet what should be passed and how. Processed message counts (added, deleted, renamed) could be passed on the command line, but how useful is that really? The same can be easily achieved through initial tagging. Message-ids could not be passed on the command line (there just can be too many of them) so it would require setting up a pipe and feeding them to stdin of the hook. The post-new hook should be run after the database has been closed, but the message-ids are not saved during notmuch new processing. Saving them for later gets complicated for not much extra benefit in addition to creative use of initial tagging, as far as I can see. Plus interrupting the post-new hook with this setup would screw up your processing if it only depended on the message-ids. All in all, I'd postpone all of this until later. BR, Jani.
[PATCH 0/3] Configuration file option to exclude files/directories
On Wed, 14 Sep 2011 00:32:01 +0300, tomi.ollila at iki.fi wrote: > This patch set adds a configuration option 'database.exclude'; a list of > files/directories to be excluded when doing 'notmuch new' operation. Hi All; I could see idea of excluding spam and trash folders from indexing being useful, but this has been sitting in the list for a few months without comment. Does that mean no-one but Tomi thinks the functionality is interesting? Or it just slipped by? d
[PATCH 2/2] python: annotate all calls into libnotmuch with types
Quoting Justus Winter (2011-12-06 13:51:08) >Quoting Sebastian Spaeth (2011-12-06 13:05:53) >>On Tue, 06 Dec 2011 10:46:31 +, Patrick Totzke >googlemail.com> wrote: >>> This commit breaks raising XapianErrors for me. >>> >>> If I lock the index with some `notmuch tag +test '*'` >>> and try to write to it in alot, i get a segfault and >>> the following on stderr: >>> >>> Xapian exception occurred opening database: Unable to get write lock on >>> /home/pazz/mail/.notmuch/xapian: already locked >> >>Hi Justus, >>I can confirm that this patch breaks as Totzke has described it: >> >>http://git.notmuchmail.org/git/notmuch/commitdiff/3434d194026ff65217d9342ffe511f67fd71e79f >> >>This change makes python segfault with a Xapianerror on stdout rather >>than the python exception that we were seeing before this patch. >> >>- _open.restype = c_void_p >>+ _open.restype = NotmuchDatabaseP >> >> >>As the patch obviously fixed other crashers I would like to not revert >>it. Can you have a look and see if you find a cause of that? > >Yes, I've seen that one as well and could not figure out what causes >it since I thought that I wasn't changing the semantic of the binding. > >I began running alot in gdb since I get segfaults within libnotmuch >from time to time and managed to get a stack trace pointing to >notmuch_database_begin_atomic, but I couldn't figure out what caused >it. #0 0x7f5f25e30f71 in notmuch_database_begin_atomic () from /home/teythoon/.local/lib/libnotmuch.so.2 #1 0x7f5f2605ef70 in ffi_call_unix64 () from /usr/lib/python2.7/lib-dynload/_ctypes.so #2 0x7f5f2605e9eb in ffi_call () from /usr/lib/python2.7/lib-dynload/_ctypes.so #3 0x7f5f260529c7 in _call_function_pointer (argcount=1, resmem=0x7fff57659500, restype=, atypes=, avalues=0x7fff576594e0, pProc=0x7f5f25e30f70 , flags=4353) at /home/packages/python/2.7/python2.7-2.7.2/Modules/_ctypes/callproc.c:827 with stderr saying: A Xapian exception occurred opening database: Unable to get write lock on /home/teythoon/Maildir/.notmuch/xapian: already locked Justus ------ next part -- A non-text attachment was scrubbed... Name: .signature Type: application/octet-stream Size: 17 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/67d23630/attachment-0001.obj>
[PATCH 0/3] Configuration file option to exclude files/directories
On Wed, 07 Dec 2011 19:51:32 -0400, David Bremner wrote: > I could see idea of excluding spam and trash folders from indexing being > useful, but this has been sitting in the list for a few months without > comment. Does that mean no-one but Tomi thinks the functionality is > interesting? Or it just slipped by? A while a go, I had hacked the code to add .git to the list of excluded dirctories, and I have since just moved .git under .notmuch. I think this ability would be useful. Tom
[PATCH 1/2] emacs: remove some code duplication in notmuch-show
David, this seems ready for push as well. Regards, Dmitry
[PATCH v4 0/3] emacs: do not call notmuch show for non-inlinable parts
Hi David. How about pushing this series? Just a humble ping :) Regards, Dmitry
[PATCH] debian: add a recommends for w3m-el or w3m-el-snapshot to notmuch-emacs
From: David Bremner Installing w3m-el provides a nicer out of the box experience for viewing html in notmuch, and the overhead is not too bad (about 5M diskspace). --- debian/control |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/debian/control b/debian/control index f6f415e..369ef07 100644 --- a/debian/control +++ b/debian/control @@ -81,6 +81,7 @@ Breaks: notmuch (<<0.6~254~) Replaces: notmuch (<<0.6~254~) Depends: ${misc:Depends}, notmuch (>= ${source:Version}), emacs23 (>= 23~) | emacs23-nox (>=23~) | emacs23-lucid (>=23~) +Recommends: w3m-el | w3m-snapshot Description: thread-based email index, search and tagging (emacs interface) Notmuch is a system for indexing, searching, reading, and tagging large collections of email messages in maildir or mh format. It uses -- 1.7.7.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Patch Amnesty Day: July 1, 2011
Hi All; As some of you know I have been making some effort to get a handle on the patches to notmuch floating around using nmbug [1]. We are doing pretty well keeping track of current patches, and I started working my way back through things that looked like patches. Sometime around the release of 0.6, this became less rewarding as most patches are either applied or stale. So I decided to draw the line there. If you are the author (or just a user) of some patches that are older than that, you should absolutely feel free to bring them to my/our attention, either by tagging them as notmuch::patch in nmbug, or by following up to the thread on the list. d [1]: http://notmuchmail.org/nmbug/ pgpjY8aHAOltQ.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] test: date_relative in notmuch search json output
On Sat, 23 Jul 2011 10:54:03 +0100, pazz wrote: > expect the date_relative field for thread entries > in notmuch search's json output > --- I pushed an amended version of this one (explaining why the date won't change in the future), and the next patch as well. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] emacs: remove some code duplication in notmuch-show
On Sat, 26 Nov 2011 02:23:30 +0400, Dmitry Kurochkin wrote: > Add optional props argument to `notmuch-show-get-header'. Use it to > get headers in `notmuch-show-insert-part-multipart/signed' and > `notmuch-show-insert-part-multipart/encrypted'. Pushed this series. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 0/3] emacs: do not call notmuch show for non-inlinable parts
On Wed, 30 Nov 2011 01:19:51 +0400, Dmitry Kurochkin wrote: > Changes: > > v4 since v3: > > * Use $(()) instead of expr(1) for math. > pushed d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: New test for exported symbols
On Mon, 28 Nov 2011 22:51:14 -0800, David Bremner wrote: > As long promised, here is a test to compare the list of symbols > retrieved with objdump to those from notmuch.sym > > I didn't verify whether this runs properly during out of tree builds. aidecoe did verify this for me on Gentoo, so I pushed these ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/3] Configuration file option to exclude files/directories
On Wed, 07 Dec 2011 19:51:32 -0400, David Bremner wrote: > I could see idea of excluding spam and trash folders from indexing being > useful, but this has been sitting in the list for a few months without > comment. Does that mean no-one but Tomi thinks the functionality is > interesting? Or it just slipped by? A while a go, I had hacked the code to add .git to the list of excluded dirctories, and I have since just moved .git under .notmuch. I think this ability would be useful. Tom ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
RFC: don't ask users to do 'sudo make install'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/12/11 15:34, David Bremner wrote: > On Wed, 07 Dec 2011 15:05:02 +0100, Rainer M Krug > wrote: > >> Because of these problems, I use checkinstall to create a deb >> file ( I am using Ubuntu) which I can then un-install again if I >> want to, or update, or anything the package manager can do. > > Note that in recent versions of notmuch, "make debian-snapshot" is > probably a better option than checkinstall, since it uses the > "official" debian packaging that ships with notmuch. Interesting. I assume, I have to do ./configure make make debian-snapshot but this resulted in the following error: dpkg-buildpackage: warning: (Use -d flag to override.) debuild: fatal error at line 1348: dpkg-buildpackage -rfakeroot -D -us -uc failed make: *** [debian-snapshot] Error 29 I also tried it with the sudo afterwards, but I got the same error, so I used checkinstall again and it worked without problems: sudo checkinstall --fstrans=yes But why did I get this error? Rainer > > I don't know how practical/useful it is to support something > similar for other distros / OSes, but we could certainly consider > it. > > d - -- Rainer M. Krug, PhD (Conservation Ecology, SUN), MSc (Conservation Biology, UCT), Dipl. Phys. (Germany) Centre of Excellence for Invasion Biology Stellenbosch University South Africa Tel : +33 - (0)9 53 10 27 44 Cell: +33 - (0)6 85 62 59 98 Fax : +33 - (0)9 58 10 27 44 Fax (D):+49 - (0)3 21 21 25 22 44 email: Rainer at krugs.de Skype: RMkrug -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk7ffg0ACgkQoYgNqgF2egqcVACfR3C2VM+RmOyxqGcG1sqTfMvJ q5gAnAvqoeYIM5OMHmL1eZAeLR6bq5KZ =jVSe -END PGP SIGNATURE-
RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 15:54:05 +0100, Rainer M Krug wrote: > Interesting. I assume, I have to do > > ./configure > make > make debian-snapshot Just "make debian-snapshot" should work if you have the packages dpkg-dev and fakeroot installed. > dpkg-buildpackage: warning: (Use -d flag to override.) > debuild: fatal error at line 1348: > dpkg-buildpackage -rfakeroot -D -us -uc failed > make: *** [debian-snapshot] Error 29 Is this all of your output? It's a pretty generic error saying dpkg-buildpackage didn't work. If there is no other output, then I guess you might be missing dpkg-buildpackage. d
Re: [PATCH 0/3] Configuration file option to exclude files/directories
On Wed, 14 Sep 2011 00:32:01 +0300, tomi.oll...@iki.fi wrote: > This patch set adds a configuration option 'database.exclude'; a list of > files/directories to be excluded when doing 'notmuch new' operation. Hi All; I could see idea of excluding spam and trash folders from indexing being useful, but this has been sitting in the list for a few months without comment. Does that mean no-one but Tomi thinks the functionality is interesting? Or it just slipped by? d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 13:25:45 +0100, Justus Winter <4winter at informatik.uni-hamburg.de> wrote: > Hey everyone, > > I just noticed that running configure says: > > > All required packages were found. You may now run the following > > commands to compile and install notmuch: > > > >make > >sudo make install > > I think that this is a very poor advice in general and we shouldn't > give it to users. I know that the default prefix is /usr/local, but > installing notmuch this way makes it very hard to uninstall it later > (there is no uninstall target). Too bad there is no better 'default' way to do things -- configure - make - make install is the way most software provides their installation sequence. But where does 'sudo make install' work by default? on Ubuntu, on new Fedoras. Where else ? The 'uninstall target' is also hard problem as it would require configured source directory (and possibly information gathered during installation to know what to uninstall) -- To do a separate uninstallation too that is installed during notmuch installation would be something I don't remember seeing done in unix environments ever... > If someone decides to install notmuch from source and later on using > his or her favorite package manager, the once installed notmuch in > /usr/local will still be found and before the one in /usr and thus be > preferred. Hmm, I've screwed my PATH setting as there usr/local/* comes after /usr/* ;/ > Maybe we should mention stow (https://www.gnu.org/s/stow/)? Hmm, in addition to mentioning it there should be step-by-step information how to proceed with it to compile and install notmuch (and all the prerequisities required) using stow. That should not be too hard -- just log the installation steps. Maybe SomeBody(TM) takes some effort there ? > > Justus Tomi
[PATCH v4 5/5] notmuch-search: convert to command-line-arguments
From: David Bremner The switch on format_sel is slightly clunky, but it doesn't seem worth special casing argument processing for function pointers, when I think the function pointer approach will be modified/abandoned. --- notmuch-search.c | 106 +++-- 1 files changed, 38 insertions(+), 68 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 36686d1..861c888 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -415,81 +415,51 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_str; -char *opt; notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; const search_format_t *format = &format_text; -int i, ret; +int opt_index, ret; output_t output = OUTPUT_SUMMARY; int offset = 0; int limit = -1; /* unlimited */ -argc--; argv++; /* skip subcommand argument */ - -for (i = 0; i < argc && argv[i][0] == '-'; i++) { - if (strcmp (argv[i], "--") == 0) { - i++; - break; - } -if (STRNCMP_LITERAL (argv[i], "--sort=") == 0) { - opt = argv[i] + sizeof ("--sort=") - 1; - if (strcmp (opt, "oldest-first") == 0) { - sort = NOTMUCH_SORT_OLDEST_FIRST; - } else if (strcmp (opt, "newest-first") == 0) { - sort = NOTMUCH_SORT_NEWEST_FIRST; - } else { - fprintf (stderr, "Invalid value for --sort: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--offset=") == 0) { - char *p; - opt = argv[i] + sizeof ("--offset=") - 1; - offset = strtol (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --offset: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--limit=") == 0) { - char *p; - opt = argv[i] + sizeof ("--limit=") - 1; - limit = strtoul (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --limit: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { - opt = argv[i] + sizeof ("--format=") - 1; - if (strcmp (opt, "text") == 0) { - format = &format_text; - } else if (strcmp (opt, "json") == 0) { - format = &format_json; - } else { - fprintf (stderr, "Invalid value for --format: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--output=") == 0) { - opt = argv[i] + sizeof ("--output=") - 1; - if (strcmp (opt, "summary") == 0) { - output = OUTPUT_SUMMARY; - } else if (strcmp (opt, "threads") == 0) { - output = OUTPUT_THREADS; - } else if (strcmp (opt, "messages") == 0) { - output = OUTPUT_MESSAGES; - } else if (strcmp (opt, "files") == 0) { - output = OUTPUT_FILES; - } else if (strcmp (opt, "tags") == 0) { - output = OUTPUT_TAGS; - } else { - fprintf (stderr, "Invalid value for --output: %s\n", opt); - return 1; - } - } else { - fprintf (stderr, "Unrecognized option: %s\n", argv[i]); - return 1; - } +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT } + format_sel = NOTMUCH_FORMAT_TEXT; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, &sort, "sort", 's', + (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST }, + { "newest-first", NOTMUCH_SORT_NEWEST_FIRST }, + { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f', + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, + { "text", NOTMUCH_FORMAT_TEXT }, + { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &output, "output", 'o', + (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, + { "threads", OUTPUT_THREADS }, + { "messages", OUTPUT_MESSAGES }, + { "files", OUTPUT_FILES }, + { "tags", OUTPUT_TAGS }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 }, + { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + return 1; } -argc -= i; -argv += i; +switch (format_sel) { +case NOTMUCH_FORMAT_TEXT: + format = &format_text; + break;
[PATCH v4 4/5] notmuch-restore: convert to command-line-arguments
From: David Bremner The new argument handling is a bit more concise, and bit more flexible. It allows the input file name to go before the --accumulate option. --- notmuch-restore.c | 37 +++-- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 13b4325..87d9772 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -18,8 +18,6 @@ * Author: Carl Worth */ -#include - #include "notmuch-client.h" int @@ -29,12 +27,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_bool_t synchronize_flags; notmuch_bool_t accumulate = FALSE; +char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; size_t line_size; ssize_t line_len; regex_t regex; int rerr; +int opt_index; config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) @@ -47,37 +47,30 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -struct option options[] = { - { "accumulate", no_argument, 0, 'a' }, - { 0, 0, 0, 0} +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_POSITION, &input_file_name, 0, 0, 0 }, + { NOTMUCH_OPT_BOOLEAN, &accumulate, "accumulate", 'a', 0 }, + { 0, 0, 0, 0, 0 } }; -int opt; -do { - opt = getopt_long (argc, argv, "", options, NULL); - - switch (opt) { - case 'a': - accumulate = 1; - break; - case '?': - return 1; - break; - } +opt_index = parse_arguments (argc, argv, options, 1); -} while (opt != -1); +if (opt_index < 0) { + /* diagnostics already printed */ + return 1; +} -if (optind < argc) { - input = fopen (argv[optind], "r"); +if (input_file_name) { + input = fopen (input_file_name, "r"); if (input == NULL) { fprintf (stderr, "Error opening %s for reading: %s\n", -argv[optind], strerror (errno)); +input_file_name, strerror (errno)); return 1; } optind++; } -if (optind < argc) { +if (opt_index < argc) { fprintf (stderr, "Cannot read dump from more than one file: %s\n", argv[optind]); -- 1.7.5.4
[PATCH v4 3/5] notmuch-dump: convert to command-line-arguments
From: David Bremner The output file is handled via positional arguments. There are currently no "normal" options. --- notmuch-dump.c | 31 +++ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/notmuch-dump.c b/notmuch-dump.c index a490917..a735875 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -41,27 +41,34 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[]) if (notmuch == NULL) return 1; -argc--; argv++; /* skip subcommand argument */ +char *output_file_name = NULL; +int opt_index; -if (argc && strcmp (argv[0], "--") != 0) { +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_POSITION, &output_file_name, 0, 0, 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + /* diagnostics already printed */ + return 1; +} + +if (output_file_name) { fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n"); - output = fopen (argv[0], "w"); + output = fopen (output_file_name, "w"); if (output == NULL) { fprintf (stderr, "Error opening %s for writing: %s\n", -argv[0], strerror (errno)); +output_file_name, strerror (errno)); return 1; } - argc--; - argv++; } -if (argc && strcmp (argv[0], "--") == 0){ - argc--; - argv++; -} -if (argc) { - query_str = query_string_from_args (notmuch, argc, argv); +if (opt_index < argc) { + query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index); if (query_str == NULL) { fprintf (stderr, "Out of memory.\n"); return 1; -- 1.7.5.4
[PATCH v4 2/5] test: tests for command-line-arguments.c
From: David Bremner This was needed because no current notmuch code exercises the NOTMUCH_OPT_STRING style arguments. --- test/Makefile.local | 11 - test/arg-test.c | 52 + test/argument-parsing | 16 +++ test/basic|2 +- test/notmuch-test |1 + 5 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 test/arg-test.c create mode 100755 test/argument-parsing diff --git a/test/Makefile.local b/test/Makefile.local index bffbbdb..6cb6c82 100644 --- a/test/Makefile.local +++ b/test/Makefile.local @@ -2,12 +2,17 @@ dir := test +extra_cflags += -I. + smtp_dummy_srcs = \ $(notmuch_compat_srcs) \ $(dir)/smtp-dummy.c smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o) +$(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a + $(call quiet,CC) -I. $^ -o $@ + $(dir)/smtp-dummy: $(smtp_dummy_modules) $(call quiet,CC) $^ -o $@ @@ -16,11 +21,13 @@ $(dir)/symbol-test: $(dir)/symbol-test.o .PHONY: test check -test-binaries: $(dir)/smtp-dummy $(dir)/symbol-test +test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test test: all test-binaries @${dir}/notmuch-test $(OPTIONS) check: test -CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test $(dir)/symbol-test.o +CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \ +$(dir)/symbol-test $(dir)/symbol-test.o \ +$(dir)/arg-test $(dir)/arg-test.o diff --git a/test/arg-test.c b/test/arg-test.c new file mode 100644 index 000..adc56e3 --- /dev/null +++ b/test/arg-test.c @@ -0,0 +1,52 @@ +#include +#include "command-line-arguments.h" + + +int main(int argc, char **argv){ + +int opt_index=1; + +int kw_val=0; +int int_val=0; +char *pos_arg1=NULL; +char *pos_arg2=NULL; +char *string_val=NULL; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, &kw_val, "keyword", 'k', + (notmuch_keyword_t []){ { "one", 1 }, + { "two", 2 }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, &int_val, "int", 'i', 0}, + { NOTMUCH_OPT_STRING, &string_val, "string", 's', 0}, + { NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0}, + { NOTMUCH_OPT_POSITION, &pos_arg2, 0,0, 0}, + { 0, 0, 0, 0, 0 } }; + +opt_index = parse_arguments(argc, argv, options, 1); + +if (opt_index < 0) + return 1; + +if (kw_val) + printf("keyword %d\n", kw_val); + +if (int_val) + printf("int %d\n", int_val); + +if (string_val) + printf("string %s\n", string_val); + +if (pos_arg1) + printf("positional arg 1 %s\n", pos_arg1); + +if (pos_arg2) + printf("positional arg 2 %s\n", pos_arg1); + + +for ( ; opt_index < argc ; opt_index ++) { + printf("non parsed arg %d = %s\n", opt_index, argv[opt_index]); +} + +return 0; +} diff --git a/test/argument-parsing b/test/argument-parsing new file mode 100755 index 000..672de0b --- /dev/null +++ b/test/argument-parsing @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +test_description="argument parsing" +. ./test-lib.sh + +test_begin_subtest "sanity check" +$TEST_DIRECTORY/arg-test pos1 --keyword=one --string=foo pos2 --int=7 > OUTPUT +cat < EXPECTED +keyword 1 +int 7 +string foo +positional arg 1 pos1 +positional arg 2 pos1 +EOF +test_expect_equal_file OUTPUT EXPECTED + +test_done diff --git a/test/basic b/test/basic index 4edf831..d6aed24 100755 --- a/test/basic +++ b/test/basic @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be run by notmuch-test' eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test) tests_in_suite=$(for i in $TESTS; do echo $i; done | sort) available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf '%f\n' | \ -sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" | \ +sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" | \ sort) test_expect_equal "$tests_in_suite" "$available" diff --git a/test/notmuch-test b/test/notmuch-test index 53ce355..d05bb38 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -48,6 +48,7 @@ TESTS=" search-folder-coherence atomicity python + argument-parsing " TESTS=${NOTMUCH_TESTS:=$TESTS} -- 1.7.5.4
[PATCH v4 1/5] command-line-arguments.[ch]: new argument parsing framework for notmuch.
From: David Bremner As we noticed when Jani kindly converted things to getopt_long, much of the work in argument parsing in notmuch is due to the the key-value style arguments like --format=(raw|json|text). The framework here provides positional arguments, simple switches, and --key=value style arguments that can take a value being an integer, a string, or one of a set of keywords. --- Makefile.local |1 + command-line-arguments.c | 155 ++ command-line-arguments.h | 80 notmuch-client.h |1 + 4 files changed, 237 insertions(+), 0 deletions(-) create mode 100644 command-line-arguments.c create mode 100644 command-line-arguments.h diff --git a/Makefile.local b/Makefile.local index 15e6d88..28e371a 100644 --- a/Makefile.local +++ b/Makefile.local @@ -295,6 +295,7 @@ clean: distclean: clean notmuch_client_srcs = \ + command-line-arguments.c\ debugger.c \ gmime-filter-reply.c\ gmime-filter-headers.c \ diff --git a/command-line-arguments.c b/command-line-arguments.c new file mode 100644 index 000..3aa87aa --- /dev/null +++ b/command-line-arguments.c @@ -0,0 +1,155 @@ +#include +#include +#include +#include "error_util.h" +#include "command-line-arguments.h" + +/* + Search the array of keywords for a given argument, assigning the + output variable to the corresponding value. Return FALSE if nothing + matches. +*/ + +static notmuch_bool_t +_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { + +notmuch_keyword_t *keywords = arg_desc->keywords; + +while (keywords->name) { + if (strcmp (arg_str, keywords->name) == 0) { + if (arg_desc->output_var) { + *((int *)arg_desc->output_var) = keywords->value; + } + return TRUE; + } + keywords++; +} +fprintf (stderr, "unknown keyword: %s\n", arg_str); +return FALSE; +} + +/* + Search for the {pos_arg_index}th position argument, return FALSE if + that does not exist. +*/ + +notmuch_bool_t +parse_position_arg (const char *arg_str, int pos_arg_index, + const notmuch_opt_desc_t *arg_desc) { + +int pos_arg_counter = 0; +while (arg_desc->opt_type != NOTMUCH_OPT_END){ + if (arg_desc->opt_type == NOTMUCH_OPT_POSITION) { + if (pos_arg_counter == pos_arg_index) { + if (arg_desc->output_var) { + *((const char **)arg_desc->output_var) = arg_str; + } + return TRUE; + } + pos_arg_counter++; + } + arg_desc++; +} +return FALSE; +} + +/* + * Search for a non-positional (i.e. starting with --) argument matching arg, + * parse a possible value, and assign to *output_var + */ + +notmuch_bool_t +parse_option (const char *arg, + const notmuch_opt_desc_t *options) { + +assert(arg); +assert(options); + +arg += 2; + +const notmuch_opt_desc_t *try = options; +while (try->opt_type != NOTMUCH_OPT_END) { + if (try->name && strncmp (arg, try->name, strlen (try->name)) == 0) { + char next = arg[strlen (try->name)]; + const char *value= arg+strlen(try->name)+1; + + char *endptr; + + /* Everything but boolean arguments (switches) needs a +* delimiter, and a non-zero length value +*/ + + if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { + if (next != '=' && next != ':') return FALSE; + if (value[0] == 0) return FALSE; + } else { + if (next != 0) return FALSE; + } + + if (try->output_var == NULL) + INTERNAL_ERROR ("output pointer NULL for option %s", try->name); + + switch (try->opt_type) { + case NOTMUCH_OPT_KEYWORD: + return _process_keyword_arg (try, value); + break; + case NOTMUCH_OPT_BOOLEAN: + *((notmuch_bool_t *)try->output_var) = TRUE; + return TRUE; + break; + case NOTMUCH_OPT_INT: + *((int *)try->output_var) = strtol (value, &endptr, 10); + return (*endptr == 0); + break; + case NOTMUCH_OPT_STRING: + *((const char **)try->output_var) = value; + return TRUE; + break; + case NOTMUCH_OPT_POSITION: + case NOTMUCH_OPT_END: + default: + INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type); + /*UNREACHED*/ + } + } + try++; +} +fprintf (stderr, "Unrecognized option: --%s\n", arg); +return FALSE; +} + +/* See command-line-arguments.h for description */ +int +parse_arguments (int argc, char **argv, +const notmuch_opt_desc_t *options, int opt_index) { + +int pos_arg_inde
No subject
So it turns out the completely untested NOTMUCH_OPT_STRING code was (surpise!) buggy. I also fixed a few remaining formatting issues, added a unit test, and some documentation (in command-line-arguements.h). I also reordered option description struct so that the two mandatory (in the sense of being non-zero) members come first.
[PATCH v3 0/2] notmuch hooks
Quoth Jani Nikula on Dec 07 at 8:05 pm: > On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince > wrote: > > On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins > finestructure.net> wrote: > > > Also, what if we make it so that the post-new hook script only runs if > > > notmuch new processes new messages? All of my post-new functions don't > > > need to be run at all if there is no new mail. > > I think the post-new hook should be run always (provided there have been > no errors). I think it might be surprising not to, and some users might > use the hook for something other than tagging. > > > Or would it make sense to pass this information to the hook somehow? > > It would, but as I wrote in id:"87mxb8kt5r.fsf at nikula.org", I think that > should come as another patch afterwards. I know I can't decide yet what > should be passed and how. Processed message counts (added, deleted, > renamed) could be passed on the command line, but how useful is that > really? The same can be easily achieved through initial tagging. I would worry about creating any hook interface that's difficult to use correctly. For example, notmuch new could be interrupted right between processing all new messages and calling the hook; then, on the next run, it'll tell the hook that nothing happened. More generally, any message counts passed to a hook can't be better than lower bounds and I don't see how you could use such information correctly in a hook. Using an initial "new" tag that you remove at the end of a hook, though, is stable. It guarantees that the hook is aware of any new messages at least once. If a hook needs a new message count, it should run notmuch count tag:new. > Message-ids could not be passed on the command line (there just can be > too many of them) so it would require setting up a pipe and feeding them > to stdin of the hook. The post-new hook should be run after the database > has been closed, but the message-ids are not saved during notmuch new > processing. Saving them for later gets complicated for not much extra > benefit in addition to creative use of initial tagging, as far as I can > see. Plus interrupting the post-new hook with this setup would screw up > your processing if it only depended on the message-ids. I think this would have to be a separate hook that runs concurrently with new, both so that new doesn't have to buffer this information and so that the majority of post-new hooks that don't need this information don't have to deal with it. Though, as with passing message counts to post-new, I worry about actually do anything correctly in such a hook in the presence of interruptions and failures. > All in all, I'd postpone all of this until later. That sounds wise.
RFC: don't ask users to do 'sudo make install'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/12/11 14:37, Tomi Ollila wrote: > On Wed, 07 Dec 2011 13:25:45 +0100, Justus Winter > <4winter at informatik.uni-hamburg.de> > wrote: >> Hey everyone, >> >> I just noticed that running configure says: >> >>> All required packages were found. You may now run the >>> following commands to compile and install notmuch: >>> >>> make sudo make install >> >> I think that this is a very poor advice in general and we >> shouldn't give it to users. I know that the default prefix is >> /usr/local, but installing notmuch this way makes it very hard to >> uninstall it later (there is no uninstall target). > > Too bad there is no better 'default' way to do things -- configure > - make - make install is the way most software provides their > installation sequence. Because of these problems, I use checkinstall to create a deb file ( I am using Ubuntu) which I can then un-install again if I want to, or update, or anything the package manager can do. Rainer > > But where does 'sudo make install' work by default? on Ubuntu, on > new Fedoras. Where else ? > > The 'uninstall target' is also hard problem as it would require > configured source directory (and possibly information gathered > during installation to know what to uninstall) -- To do a separate > uninstallation too that is installed during notmuch installation > would be something I don't remember seeing done in unix > environments ever... > >> If someone decides to install notmuch from source and later on >> using his or her favorite package manager, the once installed >> notmuch in /usr/local will still be found and before the one in >> /usr and thus be preferred. > > Hmm, I've screwed my PATH setting as there usr/local/* comes after > /usr/* ;/ > >> Maybe we should mention stow (https://www.gnu.org/s/stow/)? > > Hmm, in addition to mentioning it there should be step-by-step > information how to proceed with it to compile and install notmuch > (and all the prerequisities required) using stow. That should not > be too hard -- just log the installation steps. Maybe SomeBody(TM) > takes some effort there ? > >> >> Justus > > Tomi -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk7fco4ACgkQoYgNqgF2ego7dACfQ2qK0Jk/O4JQbpm+LGBQQCZv VDkAnAh/DcA7u1aIEBsiW3bDr6y104Rc =JGHi -END PGP SIGNATURE-
Re: [PATCH v3 0/2] notmuch hooks
On Wed, 07 Dec 2011 21:42:33 +0200, Jani Nikula wrote: > I wonder, though, whether the samples should be created only when > creating the database, or whenever the hooks directory does not exist? > The latter, I guess. Otherwise existing notmuch users would hardly get > the sample hooks. I agree, the later would be better. notmuch new should just create the hooks directory (and example scripts) if they don't already exist. I also think it's nice to have the scripts not executable to begin with, and the hook runner only executes the script if they are executable. jamie. pgptZAlT3RNZ6.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 0/2] notmuch hooks
On Wed, 07 Dec 2011 21:42:33 +0200, Jani Nikula wrote: > I wonder, though, whether the samples should be created only when > creating the database, or whenever the hooks directory does not exist? > The latter, I guess. Otherwise existing notmuch users would hardly get > the sample hooks. I agree, the later would be better. notmuch new should just create the hooks directory (and example scripts) if they don't already exist. I also think it's nice to have the scripts not executable to begin with, and the hook runner only executes the script if they are executable. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/766102ae/attachment.pgp>
Re: [PATCH] Release memory allocated by internet_address_list_parse_string()
On Wed, 07 Dec 2011 22:13:19 +0200, Tomi Ollila wrote: > g_object_unref() releases the memory of the InternetAddressList object > returned by internet_address_list_parse_string() -- when last (only) > reference is released, internet_address_list_finalize() will do cleanup. > --- > When reviewing, see also gmime-2.4.25/gmime/internet-address.c (or older) > I tested this patch by entering: notmuch show --format=mbox '*' | wc > > Note that this fixes one potential memory leak only in case --format=mbox > as _extract_email_address is called only there. There is at least one bug here -- I failed to check whether addresses is non-NULL before attempting to g_object_unref (). So please ignore that when reviewing whether g_object_unref() is the correct action here. I provide a new patch when I know whether re-check or separate the NULL test in the beginning of _extract_email_address(). > notmuch-show.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/notmuch-show.c b/notmuch-show.c > index 603992a..3abfa07 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -255,7 +255,7 @@ _extract_email_address (const void *ctx, const char *from) > email = talloc_strdup (ctx, email); > >DONE: > -/* XXX: How to free addresses here? */ > +g_object_unref (addresses); > return email; > } > > -- > 1.7.7.3 Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
RFC: don't ask users to do 'sudo make install'
Hey everyone, I just noticed that running configure says: > All required packages were found. You may now run the following > commands to compile and install notmuch: > >make >sudo make install I think that this is a very poor advice in general and we shouldn't give it to users. I know that the default prefix is /usr/local, but installing notmuch this way makes it very hard to uninstall it later (there is no uninstall target). If someone decides to install notmuch from source and later on using his or her favorite package manager, the once installed notmuch in /usr/local will still be found and before the one in /usr and thus be preferred. Maybe we should mention stow (https://www.gnu.org/s/stow/)? Justus -- next part -- A non-text attachment was scrubbed... Name: .signature Type: application/octet-stream Size: 17 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/21e4c9fe/attachment.obj>
[PATCH] Release memory allocated by internet_address_list_parse_string()
g_object_unref() releases the memory of the InternetAddressList object returned by internet_address_list_parse_string() -- when last (only) reference is released, internet_address_list_finalize() will do cleanup. --- When reviewing, see also gmime-2.4.25/gmime/internet-address.c (or older) I tested this patch by entering: notmuch show --format=mbox '*' | wc Note that this fixes one potential memory leak only in case --format=mbox as _extract_email_address is called only there. notmuch-show.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index 603992a..3abfa07 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -255,7 +255,7 @@ _extract_email_address (const void *ctx, const char *from) email = talloc_strdup (ctx, email); DONE: -/* XXX: How to free addresses here? */ +g_object_unref (addresses); return email; } -- 1.7.7.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 0/2] notmuch hooks
Quoth Jani Nikula on Dec 07 at 8:05 pm: > On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince > wrote: > > On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins > > wrote: > > > Also, what if we make it so that the post-new hook script only runs if > > > notmuch new processes new messages? All of my post-new functions don't > > > need to be run at all if there is no new mail. > > I think the post-new hook should be run always (provided there have been > no errors). I think it might be surprising not to, and some users might > use the hook for something other than tagging. > > > Or would it make sense to pass this information to the hook somehow? > > It would, but as I wrote in id:"87mxb8kt5r@nikula.org", I think that > should come as another patch afterwards. I know I can't decide yet what > should be passed and how. Processed message counts (added, deleted, > renamed) could be passed on the command line, but how useful is that > really? The same can be easily achieved through initial tagging. I would worry about creating any hook interface that's difficult to use correctly. For example, notmuch new could be interrupted right between processing all new messages and calling the hook; then, on the next run, it'll tell the hook that nothing happened. More generally, any message counts passed to a hook can't be better than lower bounds and I don't see how you could use such information correctly in a hook. Using an initial "new" tag that you remove at the end of a hook, though, is stable. It guarantees that the hook is aware of any new messages at least once. If a hook needs a new message count, it should run notmuch count tag:new. > Message-ids could not be passed on the command line (there just can be > too many of them) so it would require setting up a pipe and feeding them > to stdin of the hook. The post-new hook should be run after the database > has been closed, but the message-ids are not saved during notmuch new > processing. Saving them for later gets complicated for not much extra > benefit in addition to creative use of initial tagging, as far as I can > see. Plus interrupting the post-new hook with this setup would screw up > your processing if it only depended on the message-ids. I think this would have to be a separate hook that runs concurrently with new, both so that new doesn't have to buffer this information and so that the majority of post-new hooks that don't need this information don't have to deal with it. Though, as with passing message counts to post-new, I worry about actually do anything correctly in such a hook in the presence of interruptions and failures. > All in all, I'd postpone all of this until later. That sounds wise. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 15:54:05 +0100, Rainer M Krug wrote: > Interesting. I assume, I have to do > > ./configure > make > make debian-snapshot Just "make debian-snapshot" should work if you have the packages dpkg-dev and fakeroot installed. > dpkg-buildpackage: warning: (Use -d flag to override.) > debuild: fatal error at line 1348: > dpkg-buildpackage -rfakeroot -D -us -uc failed > make: *** [debian-snapshot] Error 29 Is this all of your output? It's a pretty generic error saying dpkg-buildpackage didn't work. If there is no other output, then I guess you might be missing dpkg-buildpackage. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 0/2] notmuch hooks
On Tue, 06 Dec 2011 18:27:27 -0800, Jameson Graef Rollins wrote: > On Tue, 6 Dec 2011 15:22:36 +0200, Jani Nikula wrote: > > Hi all, this is v3 of the notmuch hooks patches. I think this is nearing > > completion apart from final review and, most notably, tests. > > Hey, Jani. Thanks so much for these patches. I think this is a really > neat idea and it will be very useful. I already know that this will cut > out a lot of extraneous shell scripting that I'm relying on now. Thanks for your interest! > > I've been using this for some days now, and (subjective as it is) I have to > > say > > I like offlineimap being run from notmuch new "pre-new" hook much better > > than > > vice versa. Even more so for "post-new" tagging scripts. > > I think the only thing that I was not particularly clear about is that I > guess the "pre-new" and "post-new" scripts actually need to be scripts > named "pre-new" and "post-new" in the hooks directory. For some reason > I didn't quite grok that from the help pages, and was confused if I > needed to make a new "pre-new" subdirectory in the hooks directory or > what. I think just expecting a script at hook/pre-new is fine, but it > maybe could be made a little more explicit in the documentation. It's of course difficult for me to see this from someone else's point of view, but IMHO all the relevant details can be found in the man page and help. > In fact, I think the best thing would actually be for notmuch new to > make the .notmuch/hooks directory itself and to pre-fill it with "turned > off" hook scripts that contain some useful comments on how to use them. > This is what git does, and I think it's very handy, since it's all > fairly self documenting. The notmuch help could then just refer to the > fact that they're there, and point people to the comments in the various > scripts for how to use them. This is a good idea, and complimentary to the documentation. I'll add this. (I'll just have to come up with good samples. :o) I wonder, though, whether the samples should be created only when creating the database, or whenever the hooks directory does not exist? The latter, I guess. Otherwise existing notmuch users would hardly get the sample hooks. BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 1/5] command-line-arguments.[ch]: new argument parsing framework for notmuch.
From: David Bremner As we noticed when Jani kindly converted things to getopt_long, much of the work in argument parsing in notmuch is due to the the key-value style arguments like --format=(raw|json|text). The framework here provides positional arguments, simple switches, and --key=value style arguments that can take a value being an integer, a string, or one of a set of keywords. --- Makefile.local |1 + command-line-arguments.c | 155 ++ command-line-arguments.h | 80 notmuch-client.h |1 + 4 files changed, 237 insertions(+), 0 deletions(-) create mode 100644 command-line-arguments.c create mode 100644 command-line-arguments.h diff --git a/Makefile.local b/Makefile.local index 15e6d88..28e371a 100644 --- a/Makefile.local +++ b/Makefile.local @@ -295,6 +295,7 @@ clean: distclean: clean notmuch_client_srcs = \ + command-line-arguments.c\ debugger.c \ gmime-filter-reply.c\ gmime-filter-headers.c \ diff --git a/command-line-arguments.c b/command-line-arguments.c new file mode 100644 index 000..3aa87aa --- /dev/null +++ b/command-line-arguments.c @@ -0,0 +1,155 @@ +#include +#include +#include +#include "error_util.h" +#include "command-line-arguments.h" + +/* + Search the array of keywords for a given argument, assigning the + output variable to the corresponding value. Return FALSE if nothing + matches. +*/ + +static notmuch_bool_t +_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { + +notmuch_keyword_t *keywords = arg_desc->keywords; + +while (keywords->name) { + if (strcmp (arg_str, keywords->name) == 0) { + if (arg_desc->output_var) { + *((int *)arg_desc->output_var) = keywords->value; + } + return TRUE; + } + keywords++; +} +fprintf (stderr, "unknown keyword: %s\n", arg_str); +return FALSE; +} + +/* + Search for the {pos_arg_index}th position argument, return FALSE if + that does not exist. +*/ + +notmuch_bool_t +parse_position_arg (const char *arg_str, int pos_arg_index, + const notmuch_opt_desc_t *arg_desc) { + +int pos_arg_counter = 0; +while (arg_desc->opt_type != NOTMUCH_OPT_END){ + if (arg_desc->opt_type == NOTMUCH_OPT_POSITION) { + if (pos_arg_counter == pos_arg_index) { + if (arg_desc->output_var) { + *((const char **)arg_desc->output_var) = arg_str; + } + return TRUE; + } + pos_arg_counter++; + } + arg_desc++; +} +return FALSE; +} + +/* + * Search for a non-positional (i.e. starting with --) argument matching arg, + * parse a possible value, and assign to *output_var + */ + +notmuch_bool_t +parse_option (const char *arg, + const notmuch_opt_desc_t *options) { + +assert(arg); +assert(options); + +arg += 2; + +const notmuch_opt_desc_t *try = options; +while (try->opt_type != NOTMUCH_OPT_END) { + if (try->name && strncmp (arg, try->name, strlen (try->name)) == 0) { + char next = arg[strlen (try->name)]; + const char *value= arg+strlen(try->name)+1; + + char *endptr; + + /* Everything but boolean arguments (switches) needs a +* delimiter, and a non-zero length value +*/ + + if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { + if (next != '=' && next != ':') return FALSE; + if (value[0] == 0) return FALSE; + } else { + if (next != 0) return FALSE; + } + + if (try->output_var == NULL) + INTERNAL_ERROR ("output pointer NULL for option %s", try->name); + + switch (try->opt_type) { + case NOTMUCH_OPT_KEYWORD: + return _process_keyword_arg (try, value); + break; + case NOTMUCH_OPT_BOOLEAN: + *((notmuch_bool_t *)try->output_var) = TRUE; + return TRUE; + break; + case NOTMUCH_OPT_INT: + *((int *)try->output_var) = strtol (value, &endptr, 10); + return (*endptr == 0); + break; + case NOTMUCH_OPT_STRING: + *((const char **)try->output_var) = value; + return TRUE; + break; + case NOTMUCH_OPT_POSITION: + case NOTMUCH_OPT_END: + default: + INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type); + /*UNREACHED*/ + } + } + try++; +} +fprintf (stderr, "Unrecognized option: --%s\n", arg); +return FALSE; +} + +/* See command-line-arguments.h for description */ +int +parse_arguments (int argc, char **argv, +const notmuch_opt_desc_t *options, int opt_index) { + +int pos_arg_ind
[PATCH v4 5/5] notmuch-search: convert to command-line-arguments
From: David Bremner The switch on format_sel is slightly clunky, but it doesn't seem worth special casing argument processing for function pointers, when I think the function pointer approach will be modified/abandoned. --- notmuch-search.c | 106 +++-- 1 files changed, 38 insertions(+), 68 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 36686d1..861c888 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -415,81 +415,51 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_str; -char *opt; notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; const search_format_t *format = &format_text; -int i, ret; +int opt_index, ret; output_t output = OUTPUT_SUMMARY; int offset = 0; int limit = -1; /* unlimited */ -argc--; argv++; /* skip subcommand argument */ - -for (i = 0; i < argc && argv[i][0] == '-'; i++) { - if (strcmp (argv[i], "--") == 0) { - i++; - break; - } -if (STRNCMP_LITERAL (argv[i], "--sort=") == 0) { - opt = argv[i] + sizeof ("--sort=") - 1; - if (strcmp (opt, "oldest-first") == 0) { - sort = NOTMUCH_SORT_OLDEST_FIRST; - } else if (strcmp (opt, "newest-first") == 0) { - sort = NOTMUCH_SORT_NEWEST_FIRST; - } else { - fprintf (stderr, "Invalid value for --sort: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--offset=") == 0) { - char *p; - opt = argv[i] + sizeof ("--offset=") - 1; - offset = strtol (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --offset: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--limit=") == 0) { - char *p; - opt = argv[i] + sizeof ("--limit=") - 1; - limit = strtoul (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --limit: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { - opt = argv[i] + sizeof ("--format=") - 1; - if (strcmp (opt, "text") == 0) { - format = &format_text; - } else if (strcmp (opt, "json") == 0) { - format = &format_json; - } else { - fprintf (stderr, "Invalid value for --format: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--output=") == 0) { - opt = argv[i] + sizeof ("--output=") - 1; - if (strcmp (opt, "summary") == 0) { - output = OUTPUT_SUMMARY; - } else if (strcmp (opt, "threads") == 0) { - output = OUTPUT_THREADS; - } else if (strcmp (opt, "messages") == 0) { - output = OUTPUT_MESSAGES; - } else if (strcmp (opt, "files") == 0) { - output = OUTPUT_FILES; - } else if (strcmp (opt, "tags") == 0) { - output = OUTPUT_TAGS; - } else { - fprintf (stderr, "Invalid value for --output: %s\n", opt); - return 1; - } - } else { - fprintf (stderr, "Unrecognized option: %s\n", argv[i]); - return 1; - } +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT } + format_sel = NOTMUCH_FORMAT_TEXT; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, &sort, "sort", 's', + (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST }, + { "newest-first", NOTMUCH_SORT_NEWEST_FIRST }, + { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &format_sel, "format", 'f', + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, + { "text", NOTMUCH_FORMAT_TEXT }, + { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &output, "output", 'o', + (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, + { "threads", OUTPUT_THREADS }, + { "messages", OUTPUT_MESSAGES }, + { "files", OUTPUT_FILES }, + { "tags", OUTPUT_TAGS }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, &offset, "offset", 'O', 0 }, + { NOTMUCH_OPT_INT, &limit, "limit", 'L', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + return 1; } -argc -= i; -argv += i; +switch (format_sel) { +case NOTMUCH_FORMAT_TEXT: + format = &format_text; + brea
[PATCH v4 2/5] test: tests for command-line-arguments.c
From: David Bremner This was needed because no current notmuch code exercises the NOTMUCH_OPT_STRING style arguments. --- test/Makefile.local | 11 - test/arg-test.c | 52 + test/argument-parsing | 16 +++ test/basic|2 +- test/notmuch-test |1 + 5 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 test/arg-test.c create mode 100755 test/argument-parsing diff --git a/test/Makefile.local b/test/Makefile.local index bffbbdb..6cb6c82 100644 --- a/test/Makefile.local +++ b/test/Makefile.local @@ -2,12 +2,17 @@ dir := test +extra_cflags += -I. + smtp_dummy_srcs = \ $(notmuch_compat_srcs) \ $(dir)/smtp-dummy.c smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o) +$(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a + $(call quiet,CC) -I. $^ -o $@ + $(dir)/smtp-dummy: $(smtp_dummy_modules) $(call quiet,CC) $^ -o $@ @@ -16,11 +21,13 @@ $(dir)/symbol-test: $(dir)/symbol-test.o .PHONY: test check -test-binaries: $(dir)/smtp-dummy $(dir)/symbol-test +test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test test: all test-binaries @${dir}/notmuch-test $(OPTIONS) check: test -CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o $(dir)/symbol-test $(dir)/symbol-test.o +CLEAN := $(CLEAN) $(dir)/smtp-dummy $(dir)/smtp-dummy.o \ +$(dir)/symbol-test $(dir)/symbol-test.o \ +$(dir)/arg-test $(dir)/arg-test.o diff --git a/test/arg-test.c b/test/arg-test.c new file mode 100644 index 000..adc56e3 --- /dev/null +++ b/test/arg-test.c @@ -0,0 +1,52 @@ +#include +#include "command-line-arguments.h" + + +int main(int argc, char **argv){ + +int opt_index=1; + +int kw_val=0; +int int_val=0; +char *pos_arg1=NULL; +char *pos_arg2=NULL; +char *string_val=NULL; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, &kw_val, "keyword", 'k', + (notmuch_keyword_t []){ { "one", 1 }, + { "two", 2 }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, &int_val, "int", 'i', 0}, + { NOTMUCH_OPT_STRING, &string_val, "string", 's', 0}, + { NOTMUCH_OPT_POSITION, &pos_arg1, 0,0, 0}, + { NOTMUCH_OPT_POSITION, &pos_arg2, 0,0, 0}, + { 0, 0, 0, 0, 0 } }; + +opt_index = parse_arguments(argc, argv, options, 1); + +if (opt_index < 0) + return 1; + +if (kw_val) + printf("keyword %d\n", kw_val); + +if (int_val) + printf("int %d\n", int_val); + +if (string_val) + printf("string %s\n", string_val); + +if (pos_arg1) + printf("positional arg 1 %s\n", pos_arg1); + +if (pos_arg2) + printf("positional arg 2 %s\n", pos_arg1); + + +for ( ; opt_index < argc ; opt_index ++) { + printf("non parsed arg %d = %s\n", opt_index, argv[opt_index]); +} + +return 0; +} diff --git a/test/argument-parsing b/test/argument-parsing new file mode 100755 index 000..672de0b --- /dev/null +++ b/test/argument-parsing @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +test_description="argument parsing" +. ./test-lib.sh + +test_begin_subtest "sanity check" +$TEST_DIRECTORY/arg-test pos1 --keyword=one --string=foo pos2 --int=7 > OUTPUT +cat < EXPECTED +keyword 1 +int 7 +string foo +positional arg 1 pos1 +positional arg 2 pos1 +EOF +test_expect_equal_file OUTPUT EXPECTED + +test_done diff --git a/test/basic b/test/basic index 4edf831..d6aed24 100755 --- a/test/basic +++ b/test/basic @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be run by notmuch-test' eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test) tests_in_suite=$(for i in $TESTS; do echo $i; done | sort) available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf '%f\n' | \ -sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test)$/d" | \ +sed -r -e "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" | \ sort) test_expect_equal "$tests_in_suite" "$available" diff --git a/test/notmuch-test b/test/notmuch-test index 53ce355..d05bb38 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -48,6 +48,7 @@ TESTS=" search-folder-coherence atomicity python + argument-parsing " TESTS=${NOTMUCH_TESTS:=$TESTS} -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 4/5] notmuch-restore: convert to command-line-arguments
From: David Bremner The new argument handling is a bit more concise, and bit more flexible. It allows the input file name to go before the --accumulate option. --- notmuch-restore.c | 37 +++-- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 13b4325..87d9772 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -18,8 +18,6 @@ * Author: Carl Worth */ -#include - #include "notmuch-client.h" int @@ -29,12 +27,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_bool_t synchronize_flags; notmuch_bool_t accumulate = FALSE; +char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; size_t line_size; ssize_t line_len; regex_t regex; int rerr; +int opt_index; config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) @@ -47,37 +47,30 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -struct option options[] = { - { "accumulate", no_argument, 0, 'a' }, - { 0, 0, 0, 0} +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_POSITION, &input_file_name, 0, 0, 0 }, + { NOTMUCH_OPT_BOOLEAN, &accumulate, "accumulate", 'a', 0 }, + { 0, 0, 0, 0, 0 } }; -int opt; -do { - opt = getopt_long (argc, argv, "", options, NULL); - - switch (opt) { - case 'a': - accumulate = 1; - break; - case '?': - return 1; - break; - } +opt_index = parse_arguments (argc, argv, options, 1); -} while (opt != -1); +if (opt_index < 0) { + /* diagnostics already printed */ + return 1; +} -if (optind < argc) { - input = fopen (argv[optind], "r"); +if (input_file_name) { + input = fopen (input_file_name, "r"); if (input == NULL) { fprintf (stderr, "Error opening %s for reading: %s\n", -argv[optind], strerror (errno)); +input_file_name, strerror (errno)); return 1; } optind++; } -if (optind < argc) { +if (opt_index < argc) { fprintf (stderr, "Cannot read dump from more than one file: %s\n", argv[optind]); -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 3/5] notmuch-dump: convert to command-line-arguments
From: David Bremner The output file is handled via positional arguments. There are currently no "normal" options. --- notmuch-dump.c | 31 +++ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/notmuch-dump.c b/notmuch-dump.c index a490917..a735875 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -41,27 +41,34 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[]) if (notmuch == NULL) return 1; -argc--; argv++; /* skip subcommand argument */ +char *output_file_name = NULL; +int opt_index; -if (argc && strcmp (argv[0], "--") != 0) { +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_POSITION, &output_file_name, 0, 0, 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + /* diagnostics already printed */ + return 1; +} + +if (output_file_name) { fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n"); - output = fopen (argv[0], "w"); + output = fopen (output_file_name, "w"); if (output == NULL) { fprintf (stderr, "Error opening %s for writing: %s\n", -argv[0], strerror (errno)); +output_file_name, strerror (errno)); return 1; } - argc--; - argv++; } -if (argc && strcmp (argv[0], "--") == 0){ - argc--; - argv++; -} -if (argc) { - query_str = query_string_from_args (notmuch, argc, argv); +if (opt_index < argc) { + query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index); if (query_str == NULL) { fprintf (stderr, "Out of memory.\n"); return 1; -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[no subject]
So it turns out the completely untested NOTMUCH_OPT_STRING code was (surpise!) buggy. I also fixed a few remaining formatting issues, added a unit test, and some documentation (in command-line-arguements.h). I also reordered option description struct so that the two mandatory (in the sense of being non-zero) members come first. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/2] python: annotate all calls into libnotmuch with types
Quoting Justus Winter (2011-12-07 19:49:31) >Quoting Justus Winter (2011-12-06 13:51:08) >>I began running alot in gdb since I get segfaults within libnotmuch >>from time to time and managed to get a stack trace pointing to >>notmuch_database_begin_atomic, but I couldn't figure out what caused >>it. And another one: Reading symbols from /usr/bin/python...Reading symbols from /usr/lib/debug/usr/bin/python2.7...done. done. [New LWP 15188] [Thread debugging using libthread_db enabled] Core was generated by `/usr/bin/python /home/teythoon/.local/bin/afew -v --tag --new'. Program terminated with signal 6, Aborted. #0 0x7f72f2cce405 in *__GI_raise (sig=) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) bt #0 0x7f72f2cce405 in *__GI_raise (sig=) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x7f72f2cd1680 in *__GI_abort () at abort.c:92 #2 0x7f72f1380bfd in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #3 0x7f72f137eda6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #4 0x7f72f137edd3 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #5 0x7f72f137eece in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6 #6 0x7f72f16b792e in ChertTable::set_overwritten() const () from /usr/lib/libxapian.so.22 #7 0x7f72f16ba7a6 in ChertTable::block_to_cursor(Cursor*, int, unsigned int) const () from /usr/lib/libxapian.so.22 #8 0x7f72f16bcc35 in ChertTable::find(Cursor*) const () from /usr/lib/libxapian.so.22 #9 0x7f72f1697431 in ChertCursor::find_entry(std::string const&) () from /usr/lib/libxapian.so.22 #10 0x7f72f16c0858 in ?? () from /usr/lib/libxapian.so.22 #11 0x7f72f16c0c69 in ?? () from /usr/lib/libxapian.so.22 #12 0x7f72f16a649a in ?? () from /usr/lib/libxapian.so.22 #13 0x7f72f1621001 in Xapian::Document::Internal::get_value(unsigned int) const () from /usr/lib/libxapian.so.22 #14 0x7f72f162103c in Xapian::Document::get_value(unsigned int) const () from /usr/lib/libxapian.so.22 #15 0x7f72f238ef23 in notmuch_message_get_header () from /home/teythoon/.local/lib/libnotmuch.so.2 #16 0x7f72f25b7f70 in ffi_call_unix64 () from /usr/lib/python2.7/lib-dynload/_ctypes.so #17 0x7f72f25b79eb in ffi_call () from /usr/lib/python2.7/lib-dynload/_ctypes.so #18 0x7f72f25ab9c7 in _call_function_pointer (argcount=2, resmem=0x7fffc13a0310, restype=, atypes=, avalues=0x7fffc13a02f0, pProc=0x7f72f238ee50 , flags=4353) at /home/packages/python/2.7/python2.7-2.7.2/Modules/_ctypes/callproc.c:827 #19 _ctypes_callproc (pProc=0x7f72f238ee50 , argtuple=0x0, flags=4353, argtypes=, restype= <_ctypes.PyCSimpleType at remote 0xd17c30>, checker=0x0) at /home/packages/python/2.7/python2.7-2.7.2/Modules/_ctypes/callproc.c:1174 #20 0x7f72f25a3257 in PyCFuncPtr_call (self=, inargs=, kwds=0x0) at /home/packages/python/2.7/python2.7-2.7.2/Modules/_ctypes/_ctypes.c:3913 #21 0x0041d35a in PyObject_Call (func=<_FuncPtr(__name__='notmuch_message_get_header') at remote 0xe02530>, arg=, kw=) at ../Objects/abstract.c:2529 #22 0x004b9b4e in do_call (nk=, na=, pp_stack=0x7fffc13a05f0, func= <_FuncPtr(__name__='notmuch_message_get_header') at remote 0xe02530>) at ../Python/ceval.c:4239 #23 call_function (oparg=, pp_stack=0x7fffc13a05f0) at ../Python/ceval.c:4044 #24 PyEval_EvalFrameEx (f=, throwflag=) at ../Python/ceval.c:2666 #25 0x004b9d27 in fast_function (nk=, na=, n=, pp_stack=0x7fffc13a0730, func=) at ../Python/ceval.c:4107 #26 call_function (oparg=, pp_stack=0x7fffc13a0730) at ../Python/ceval.c:4042 #27 PyEval_EvalFrameEx (f=, throwflag=) at ../Python/ceval.c:2666 #28 0x004bfc9d in PyEval_EvalCodeEx (co=0xcd28b0, globals=, locals=, args=, argcount=, kws=, kwcount=0, defs=0x0, defcount=0, closure=0x0) at ../Python/ceval.c:3253 #29 0x0044b36f in function_call (func=, arg= () at remote 0xe6c9d0>, _query=) at remote 0xe6ca50>, _msgs=) at remote 0xe6c990>, _msg=) at remote 0x11a1b10>,), kw=0x0) at ../Objects/funcobject.c:526 #30 0x0041d35a in PyObject_Call (func=, arg=, kw=) at ../Objects/abstract.c:2529 #31 0x00432bdb in instancemethod_call (func=, arg= () at remote 0xe6c9d0>, _query=) at remote 0xe6ca50>, _msgs=) at remote 0xe6c990>, _msg=) at remote 0x11a1b10>,), kw=0x0) at ../Objects/classobject.c:2578 #32 0x0042668c in PyObject_Call (func=, arg=, kw=0x0) at ../Objects/abstract.c:2529 #33 0x004299fc in PyObject_CallFunctionObjArgs (callable=) at ../Objects/abstract.c:2760 #34 0x00466ebb in PyObject_Unicode (v= ) at remote 0xe6c9d0>, _query=) at remote 0xe6ca50>, _msgs=) at remote 0xe6c990>, _msg=) at remote 0x11a1b10>) at ../Objects/object.c:509 #35 0x00494443 in unicode_new (type=0x853200, args=, kwds=) at ../Objects/unicodeobject.c:8722 #36 0x0047ef15
Re: [PATCH 2/2] python: annotate all calls into libnotmuch with types
Quoting Justus Winter (2011-12-06 13:51:08) >Quoting Sebastian Spaeth (2011-12-06 13:05:53) >>On Tue, 06 Dec 2011 10:46:31 +, Patrick Totzke >> wrote: >>> This commit breaks raising XapianErrors for me. >>> >>> If I lock the index with some `notmuch tag +test '*'` >>> and try to write to it in alot, i get a segfault and >>> the following on stderr: >>> >>> Xapian exception occurred opening database: Unable to get write lock on >>> /home/pazz/mail/.notmuch/xapian: already locked >> >>Hi Justus, >>I can confirm that this patch breaks as Totzke has described it: >> >>http://git.notmuchmail.org/git/notmuch/commitdiff/3434d194026ff65217d9342ffe511f67fd71e79f >> >>This change makes python segfault with a Xapianerror on stdout rather >>than the python exception that we were seeing before this patch. >> >>- _open.restype = c_void_p >>+ _open.restype = NotmuchDatabaseP >> >> >>As the patch obviously fixed other crashers I would like to not revert >>it. Can you have a look and see if you find a cause of that? > >Yes, I've seen that one as well and could not figure out what causes >it since I thought that I wasn't changing the semantic of the binding. > >I began running alot in gdb since I get segfaults within libnotmuch >from time to time and managed to get a stack trace pointing to >notmuch_database_begin_atomic, but I couldn't figure out what caused >it. #0 0x7f5f25e30f71 in notmuch_database_begin_atomic () from /home/teythoon/.local/lib/libnotmuch.so.2 #1 0x7f5f2605ef70 in ffi_call_unix64 () from /usr/lib/python2.7/lib-dynload/_ctypes.so #2 0x7f5f2605e9eb in ffi_call () from /usr/lib/python2.7/lib-dynload/_ctypes.so #3 0x7f5f260529c7 in _call_function_pointer (argcount=1, resmem=0x7fff57659500, restype=, atypes=, avalues=0x7fff576594e0, pProc=0x7f5f25e30f70 , flags=4353) at /home/packages/python/2.7/python2.7-2.7.2/Modules/_ctypes/callproc.c:827 with stderr saying: A Xapian exception occurred opening database: Unable to get write lock on /home/teythoon/Maildir/.notmuch/xapian: already locked Justus .signature Description: Binary data ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] test: use python2 instead of python if available
Some distros (Arch Linux) ship Python as python2 and Python 3 as python. Checking for python2 is necessary for the Python tests to work on these platforms. --- test/test-lib.sh |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/test/test-lib.sh b/test/test-lib.sh index 519bd84..155ad3c 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -923,8 +923,14 @@ test_python() { export LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib export PYTHONPATH=$TEST_DIRECTORY/../bindings/python + # Some distros (e.g. Arch Linux) ship Python 2.* as /usr/bin/python2, + # most others as /usr/bin/python. So first try python2, and fallback to + # python if python2 doesn't exist. + cmd=python2 + [[ "$test_missing_external_prereq_python2_" = t ]] && cmd=python + (echo "import sys; _orig_stdout=sys.stdout; sys.stdout=open('OUTPUT', 'w')"; cat) \ - | python - + | $cmd - } test_reset_state_ () { @@ -1157,3 +1163,4 @@ test_declare_external_prereq emacsclient test_declare_external_prereq gdb test_declare_external_prereq gpg test_declare_external_prereq python +test_declare_external_prereq python2 -- 1.7.8
[PATCH 1/2] test: add a function to run Python tests
The new test_python() function makes writing Python tests a little easier: - it sets the environment variables as needed - it redirects stdout to the OUTPUT file (like test_emacs()). This commit also declares python as an external prereq. The stdout redirection is required to avoid trouble when running commands like "python 'script' | sort > OUTPUT": in such a case, any error due to a missing external prereq would be "swallowed" by sort, resulting to a failed test instead of a skipped one. --- test/python |6 ++ test/test-lib.sh |9 + 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/test/python b/test/python index f737749..c3aa726 100755 --- a/test/python +++ b/test/python @@ -5,9 +5,7 @@ test_description="python bindings" add_email_corpus test_begin_subtest "compare thread ids" -LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib \ -PYTHONPATH=$TEST_DIRECTORY/../bindings/python \ -python < OUTPUT +test_python < EXPECTED -test_expect_equal_file OUTPUT EXPECTED +test_expect_equal_file <(sort OUTPUT) EXPECTED test_done diff --git a/test/test-lib.sh b/test/test-lib.sh index a975957..519bd84 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -919,6 +919,14 @@ test_emacs () { emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)" } +test_python() { + export LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib + export PYTHONPATH=$TEST_DIRECTORY/../bindings/python + + (echo "import sys; _orig_stdout=sys.stdout; sys.stdout=open('OUTPUT', 'w')"; cat) \ + | python - +} + test_reset_state_ () { test -z "$test_init_done_" && test_init_ @@ -1148,3 +1156,4 @@ test_declare_external_prereq emacs test_declare_external_prereq emacsclient test_declare_external_prereq gdb test_declare_external_prereq gpg +test_declare_external_prereq python -- 1.7.8
RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 15:05:02 +0100, Rainer M Krug wrote: > Because of these problems, I use checkinstall to create a deb file ( I > am using Ubuntu) which I can then un-install again if I want to, or > update, or anything the package manager can do. Note that in recent versions of notmuch, "make debian-snapshot" is probably a better option than checkinstall, since it uses the "official" debian packaging that ships with notmuch. I don't know how practical/useful it is to support something similar for other distros / OSes, but we could certainly consider it. d
Re: [PATCH v3 0/2] notmuch hooks
On Wed, 07 Dec 2011 20:05:22 +0200, Jani Nikula wrote: > It would, but as I wrote in id:"87mxb8kt5r@nikula.org", I think that > should come as another patch afterwards. I know I can't decide yet what > should be passed and how. Processed message counts (added, deleted, > renamed) could be passed on the command line, but how useful is that > really? The same can be easily achieved through initial tagging. I would say that probably the best way to pass information to the hook scripts is via the environment. For instance, notmuch could pass the variable NOTMUCH_NEW_MESSAGES containing the number of new messages to the post-new script. jamie. pgpoG5ZoAjlGX.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 0/2] notmuch hooks
On Wed, 07 Dec 2011 20:05:22 +0200, Jani Nikula wrote: > It would, but as I wrote in id:"87mxb8kt5r.fsf at nikula.org", I think that > should come as another patch afterwards. I know I can't decide yet what > should be passed and how. Processed message counts (added, deleted, > renamed) could be passed on the command line, but how useful is that > really? The same can be easily achieved through initial tagging. I would say that probably the best way to pass information to the hook scripts is via the environment. For instance, notmuch could pass the variable NOTMUCH_NEW_MESSAGES containing the number of new messages to the post-new script. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20111207/f62dcb96/attachment.pgp>
Re: [PATCH v3 0/2] notmuch hooks
On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince wrote: > On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins > wrote: > > Also, what if we make it so that the post-new hook script only runs if > > notmuch new processes new messages? All of my post-new functions don't > > need to be run at all if there is no new mail. I think the post-new hook should be run always (provided there have been no errors). I think it might be surprising not to, and some users might use the hook for something other than tagging. > Or would it make sense to pass this information to the hook somehow? It would, but as I wrote in id:"87mxb8kt5r@nikula.org", I think that should come as another patch afterwards. I know I can't decide yet what should be passed and how. Processed message counts (added, deleted, renamed) could be passed on the command line, but how useful is that really? The same can be easily achieved through initial tagging. Message-ids could not be passed on the command line (there just can be too many of them) so it would require setting up a pipe and feeding them to stdin of the hook. The post-new hook should be run after the database has been closed, but the message-ids are not saved during notmuch new processing. Saving them for later gets complicated for not much extra benefit in addition to creative use of initial tagging, as far as I can see. Plus interrupting the post-new hook with this setup would screw up your processing if it only depended on the message-ids. All in all, I'd postpone all of this until later. BR, Jani. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/4] notmuch-search: convert to command-line-arguments
From: David Bremner The switch on format_sel is slightly clunky, but it doesn't seem worth special casing argument processing for function pointers, when I think the function pointer approach will be modified/abandoned. --- notmuch-search.c | 109 - 1 files changed, 41 insertions(+), 68 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 36686d1..2ae7597 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -415,81 +415,54 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_str; -char *opt; notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; const search_format_t *format = &format_text; -int i, ret; +int opt_index, ret; output_t output = OUTPUT_SUMMARY; int offset = 0; int limit = -1; /* unlimited */ -argc--; argv++; /* skip subcommand argument */ - -for (i = 0; i < argc && argv[i][0] == '-'; i++) { - if (strcmp (argv[i], "--") == 0) { - i++; - break; - } -if (STRNCMP_LITERAL (argv[i], "--sort=") == 0) { - opt = argv[i] + sizeof ("--sort=") - 1; - if (strcmp (opt, "oldest-first") == 0) { - sort = NOTMUCH_SORT_OLDEST_FIRST; - } else if (strcmp (opt, "newest-first") == 0) { - sort = NOTMUCH_SORT_NEWEST_FIRST; - } else { - fprintf (stderr, "Invalid value for --sort: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--offset=") == 0) { - char *p; - opt = argv[i] + sizeof ("--offset=") - 1; - offset = strtol (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --offset: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--limit=") == 0) { - char *p; - opt = argv[i] + sizeof ("--limit=") - 1; - limit = strtoul (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --limit: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { - opt = argv[i] + sizeof ("--format=") - 1; - if (strcmp (opt, "text") == 0) { - format = &format_text; - } else if (strcmp (opt, "json") == 0) { - format = &format_json; - } else { - fprintf (stderr, "Invalid value for --format: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--output=") == 0) { - opt = argv[i] + sizeof ("--output=") - 1; - if (strcmp (opt, "summary") == 0) { - output = OUTPUT_SUMMARY; - } else if (strcmp (opt, "threads") == 0) { - output = OUTPUT_THREADS; - } else if (strcmp (opt, "messages") == 0) { - output = OUTPUT_MESSAGES; - } else if (strcmp (opt, "files") == 0) { - output = OUTPUT_FILES; - } else if (strcmp (opt, "tags") == 0) { - output = OUTPUT_TAGS; - } else { - fprintf (stderr, "Invalid value for --output: %s\n", opt); - return 1; - } - } else { - fprintf (stderr, "Unrecognized option: %s\n", argv[i]); - return 1; - } +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT } + format_sel = NOTMUCH_FORMAT_TEXT; + +notmuch_opt_desc_t options[] = { + { "sort", 's', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST }, + { "newest-first", NOTMUCH_SORT_NEWEST_FIRST }, + {0, 0} }, + &sort }, + { "format", 'f', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, + { "text", NOTMUCH_FORMAT_TEXT }, + {0, 0} }, + &format_sel }, + { "output", 'o', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, + { "threads", OUTPUT_THREADS }, + { "messages", OUTPUT_MESSAGES }, + { "files", OUTPUT_FILES }, + { "tags", OUTPUT_TAGS }, + {0, 0} }, + &output }, + { "offset", 'O', NOTMUCH_OPT_INT, 0, &offset }, + { "limit", 'L', NOTMUCH_OPT_INT, 0, &limit }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + exit(1); } -argc -= i; -argv += i; +switch (format_sel) { +case NOTMUCH_FORMAT_TEXT: + format = &forma
[PATCH 3/4] notmuch-restore: convert to command-line-arguments
From: David Bremner The new argument handling is a bit more concise, and bit more flexible. It allows the input file name to go before the --accumulate option. --- notmuch-restore.c | 37 +++-- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 13b4325..708f3e9 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -18,8 +18,6 @@ * Author: Carl Worth */ -#include - #include "notmuch-client.h" int @@ -29,12 +27,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_bool_t synchronize_flags; notmuch_bool_t accumulate = FALSE; +char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; size_t line_size; ssize_t line_len; regex_t regex; int rerr; +int opt_index; config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) @@ -47,37 +47,30 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -struct option options[] = { - { "accumulate", no_argument, 0, 'a' }, - { 0, 0, 0, 0} +notmuch_opt_desc_t options[] = { + { "in-file", 'i', NOTMUCH_OPT_POSITION, 0, &input_file_name }, + { "accumulate", 'a', NOTMUCH_OPT_BOOLEAN, 0, &accumulate }, + { 0, 0, 0, 0, 0 } }; -int opt; -do { - opt = getopt_long (argc, argv, "", options, NULL); +opt_index = parse_arguments (argc, argv, options, 1); - switch (opt) { - case 'a': - accumulate = 1; - break; - case '?': - return 1; - break; - } - -} while (opt != -1); +if (opt_index < 0) { + /* diagnostics already printed */ + exit(1); +} -if (optind < argc) { - input = fopen (argv[optind], "r"); +if (input_file_name) { + input = fopen (input_file_name, "r"); if (input == NULL) { fprintf (stderr, "Error opening %s for reading: %s\n", -argv[optind], strerror (errno)); +input_file_name, strerror (errno)); return 1; } optind++; } -if (optind < argc) { +if (opt_index < argc) { fprintf (stderr, "Cannot read dump from more than one file: %s\n", argv[optind]); -- 1.7.7.3
[PATCH 2/4] notmuch-dump: convert to command-line-arguments
From: David Bremner The output file is handled via positional arguments. There are currently no "normal" options. --- notmuch-dump.c | 31 +++ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/notmuch-dump.c b/notmuch-dump.c index a490917..c568734 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -41,27 +41,34 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[]) if (notmuch == NULL) return 1; -argc--; argv++; /* skip subcommand argument */ +char *output_file_name = NULL; +int opt_index; -if (argc && strcmp (argv[0], "--") != 0) { +notmuch_opt_desc_t options[] = { + { "out-file", 'o', NOTMUCH_OPT_POSITION, 0, &output_file_name }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + /* diagnostics already printed */ + exit(1); +} + +if (output_file_name) { fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n"); - output = fopen (argv[0], "w"); + output = fopen (output_file_name, "w"); if (output == NULL) { fprintf (stderr, "Error opening %s for writing: %s\n", -argv[0], strerror (errno)); +output_file_name, strerror (errno)); return 1; } - argc--; - argv++; } -if (argc && strcmp (argv[0], "--") == 0){ - argc--; - argv++; -} -if (argc) { - query_str = query_string_from_args (notmuch, argc, argv); +if (opt_index < argc) { + query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index); if (query_str == NULL) { fprintf (stderr, "Out of memory.\n"); return 1; -- 1.7.7.3
[PATCH 1/4] command-line-arguments.[ch]: new argument parsing framework for notmuch.
From: David Bremner As we noticed when Jani kindly converted things to getopt_long, much of the work in argument parsing in notmuch is due to the the key-value style arguments like --format=(raw|json|text). The framework here provides positional arguments, simple switches, and --key=value style arguments that can take a value being an integer, a string, or one of a set of keywords. --- Makefile.local |1 + command-line-arguments.c | 159 ++ command-line-arguments.h | 39 +++ notmuch-client.h |1 + 4 files changed, 200 insertions(+), 0 deletions(-) create mode 100644 command-line-arguments.c create mode 100644 command-line-arguments.h diff --git a/Makefile.local b/Makefile.local index 15e6d88..28e371a 100644 --- a/Makefile.local +++ b/Makefile.local @@ -295,6 +295,7 @@ clean: distclean: clean notmuch_client_srcs = \ + command-line-arguments.c\ debugger.c \ gmime-filter-reply.c\ gmime-filter-headers.c \ diff --git a/command-line-arguments.c b/command-line-arguments.c new file mode 100644 index 000..96ed9ba --- /dev/null +++ b/command-line-arguments.c @@ -0,0 +1,159 @@ +#include +#include +#include +#include "error_util.h" +#include "command-line-arguments.h" + +/* + Search the array of keywords for a given argument, assigning the + output variable to the corresponding value. Return FALSE if nothing + matches. +*/ + +static notmuch_bool_t +_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { + +notmuch_keyword_t *keywords = arg_desc->keywords; + +while (keywords->name) { + if (strcmp (arg_str, keywords->name) == 0) { + if (arg_desc->output_var) { + *((int *)arg_desc->output_var) = keywords->value; + } + return TRUE; + } + keywords++; +} +fprintf (stderr, "unknown keyword: %s\n", arg_str); +return FALSE; +} + +/* + Search for the {pos_arg_index}th position argument, return FALSE if + that does not exist. +*/ + +notmuch_bool_t +parse_position_arg (const char *arg_str, int pos_arg_index, const notmuch_opt_desc_t *arg_desc) { + +int pos_arg_counter = 0; +while (arg_desc->name){ + if (arg_desc->opt_type == NOTMUCH_OPT_POSITION) { + if (pos_arg_counter == pos_arg_index) { + if (arg_desc->output_var) { + *((const char **)arg_desc->output_var) = arg_str; + } + return TRUE; + } + pos_arg_counter++; + } + arg_desc++; +} +return FALSE; +} + +notmuch_bool_t +parse_option (const char *arg, + const notmuch_opt_desc_t *options) { + +assert(arg); +assert(options); + +arg += 2; + +const notmuch_opt_desc_t *try = options; +while (try->name) { + if (strncmp (arg, try->name, strlen (try->name)) == 0) { + char next = arg[strlen (try->name)]; + const char *value= arg+strlen(try->name)+1; + + char *endptr; + + /* Everything but boolean arguments (switches) needs a +* delimiter, and a non-zero length value +*/ + + if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { + if (next != '=' && next != ':') return FALSE; + if (value[0] == 0) return FALSE; + } else { + if (next != 0) return FALSE; + } + + if (try->output_var == NULL) + INTERNAL_ERROR ("output pointer NULL for option %s", try->name); + + switch (try->opt_type) { + case NOTMUCH_OPT_KEYWORD: + return _process_keyword_arg (try, value); + break; + case NOTMUCH_OPT_BOOLEAN: + *((notmuch_bool_t *)try->output_var) = TRUE; + return TRUE; + break; + case NOTMUCH_OPT_INT: + *((int *)try->output_var) = strtol (value, &endptr, 10); + return (*endptr == 0); + break; + case NOTMUCH_OPT_STRING: + *((const char **)try->output_var) = value; + case NOTMUCH_OPT_POSITION: + case NOTMUCH_OPT_NULL: + default: + INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type); + /*UNREACHED*/ + } + } + try++; +} +fprintf (stderr, "Unrecognized option: --%s\n", arg); +return FALSE; +} + +/* + Parse command line arguments according to structure options, + starting at position opt_index. + + All output of parsed values is via pointers in options. + + Parsing stops at -- (consumed) or at the (k+1)st argument + not starting with -- (a "positional argument") if options contains + k positional argument descriptors. + + Returns the index of first non-parsed argument, or -1 in case of error. + + */ +int +parse_arguments (int argc, char **argv, +
[PATCH 1/4] notmuch-opts.[ch]: new argument parsing framework for notmuch.
On Tue, 06 Dec 2011 22:41:23 +0200, Jani Nikula wrote: > > We chatted about reserving notmuch-*.c to notmuch commands, but I guess > it was only after you sent these. I think it would make sense though. renamed to "command-line-arguments.[ch]". Hard luck for those of you on 8.3 file systems. > > +/* > > + Search the list of keywords for a given argument, assigning the > > + output variable to the corresponding value. Return FALSE if nothing > > + matches. > > +*/ > > Array of keywords to be really pedantic. Done. > > + > > +/* skip delimiter */ > > +arg_str++; > > I think the caller should check and skip the delimiters. See my comments > below where this gets called. done, and error checking tightened up. > > So if ->output_var is NULL, the parameter is accepted but silently > ignored? I'm not sure if I should consider this a feature or a bug. :) >From one extreme to another, it is now an INTERNAL_ERROR to have output_var NULL. I couldn't see a use case for silently ignoring command line arguments. > > +parse_option (const char *arg, > > + const notmuch_opt_desc_t *options){ > > Missing space between ) and {. done. but you missed some other missing spaces ;). > I think here you should check that arg[strlen(try->name)] is '=' or ':' > for NOTMUCH_OPT_KEYWORD and NOTMUCH_OPT_INT, and '\0' otherwise. After > the check, you could pass just the value part to _process_keyword_arg(). done. > I can't figure out if and how you handle arguments with arbitrary string > values (for example --file=/path/to/file). You do specify --in-file and > --out-file in later patches, but those are with NOTMUCH_OPT_POSITION, there is now NOTMUCH_OPT_STRING which does this, but it is untested as notmuch doesn't take these kind of arguments at the moment (restore --match is one example, but those patches are unmerged so far). > I'm not sure if much weight should be put to getopt_long() > compatibility, but it accepts "--parameter value" as well as > "--parameter=value". Yeah, maybe I shouldn't let the implementation drive things this much, but having one argmument per element of argv simplfies things nicely. For now, I will skip this. > > I think notmuch_parse_args() is a complicated function enough to warrant > a proper description here. Done. > > > +int > > +notmuch_parse_args (int argc, char **argv, const notmuch_opt_desc_t > > *options, int opt_index){ > > Missing space between ) and {. Done. > > +typedef struct notmuch_opt { > > +int arg_id; > > +int keyword_id; > > +const char *string; > > +} notmuch_opt_t; > > You're not using this for anything, are you? Oops, deleted. > > > + > > +notmuch_bool_t > > +parse_option (const char *arg, const notmuch_opt_desc_t* options); > > + > > +notmuch_bool_t > > +parse_position_arg (const char *arg, > > + int position_arg_index, > > + const notmuch_opt_desc_t* options); > > Is there a good reason the above are not static? I had in mind to provide the user with the option of a getopt style loop if the loop in parse_arguments was not flexible enough. I could leave them as static until such a need arises, I suppose. Thanks for the review! Updated series to follow. d
Re: [PATCH 1/2] emacs: remove some code duplication in notmuch-show
David, this seems ready for push as well. Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 0/3] emacs: do not call notmuch show for non-inlinable parts
Hi David. How about pushing this series? Just a humble ping :) Regards, Dmitry ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 15:05:02 +0100, Rainer M Krug wrote: > Because of these problems, I use checkinstall to create a deb file ( I > am using Ubuntu) which I can then un-install again if I want to, or > update, or anything the package manager can do. Note that in recent versions of notmuch, "make debian-snapshot" is probably a better option than checkinstall, since it uses the "official" debian packaging that ships with notmuch. I don't know how practical/useful it is to support something similar for other distros / OSes, but we could certainly consider it. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: RFC: don't ask users to do 'sudo make install'
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/12/11 14:37, Tomi Ollila wrote: > On Wed, 07 Dec 2011 13:25:45 +0100, Justus Winter > <4win...@informatik.uni-hamburg.de> > wrote: >> Hey everyone, >> >> I just noticed that running configure says: >> >>> All required packages were found. You may now run the >>> following commands to compile and install notmuch: >>> >>> make sudo make install >> >> I think that this is a very poor advice in general and we >> shouldn't give it to users. I know that the default prefix is >> /usr/local, but installing notmuch this way makes it very hard to >> uninstall it later (there is no uninstall target). > > Too bad there is no better 'default' way to do things -- configure > - make - make install is the way most software provides their > installation sequence. Because of these problems, I use checkinstall to create a deb file ( I am using Ubuntu) which I can then un-install again if I want to, or update, or anything the package manager can do. Rainer > > But where does 'sudo make install' work by default? on Ubuntu, on > new Fedoras. Where else ? > > The 'uninstall target' is also hard problem as it would require > configured source directory (and possibly information gathered > during installation to know what to uninstall) -- To do a separate > uninstallation too that is installed during notmuch installation > would be something I don't remember seeing done in unix > environments ever... > >> If someone decides to install notmuch from source and later on >> using his or her favorite package manager, the once installed >> notmuch in /usr/local will still be found and before the one in >> /usr and thus be preferred. > > Hmm, I've screwed my PATH setting as there usr/local/* comes after > /usr/* ;/ > >> Maybe we should mention stow (https://www.gnu.org/s/stow/)? > > Hmm, in addition to mentioning it there should be step-by-step > information how to proceed with it to compile and install notmuch > (and all the prerequisities required) using stow. That should not > be too hard -- just log the installation steps. Maybe SomeBody(TM) > takes some effort there ? > >> >> Justus > > Tomi -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk7fco4ACgkQoYgNqgF2ego7dACfQ2qK0Jk/O4JQbpm+LGBQQCZv VDkAnAh/DcA7u1aIEBsiW3bDr6y104Rc =JGHi -END PGP SIGNATURE- ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: RFC: don't ask users to do 'sudo make install'
On Wed, 07 Dec 2011 13:25:45 +0100, Justus Winter <4win...@informatik.uni-hamburg.de> wrote: > Hey everyone, > > I just noticed that running configure says: > > > All required packages were found. You may now run the following > > commands to compile and install notmuch: > > > >make > >sudo make install > > I think that this is a very poor advice in general and we shouldn't > give it to users. I know that the default prefix is /usr/local, but > installing notmuch this way makes it very hard to uninstall it later > (there is no uninstall target). Too bad there is no better 'default' way to do things -- configure - make - make install is the way most software provides their installation sequence. But where does 'sudo make install' work by default? on Ubuntu, on new Fedoras. Where else ? The 'uninstall target' is also hard problem as it would require configured source directory (and possibly information gathered during installation to know what to uninstall) -- To do a separate uninstallation too that is installed during notmuch installation would be something I don't remember seeing done in unix environments ever... > If someone decides to install notmuch from source and later on using > his or her favorite package manager, the once installed notmuch in > /usr/local will still be found and before the one in /usr and thus be > preferred. Hmm, I've screwed my PATH setting as there usr/local/* comes after /usr/* ;/ > Maybe we should mention stow (https://www.gnu.org/s/stow/)? Hmm, in addition to mentioning it there should be step-by-step information how to proceed with it to compile and install notmuch (and all the prerequisities required) using stow. That should not be too hard -- just log the installation steps. Maybe SomeBody(TM) takes some effort there ? > > Justus Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] notmuch-restore: convert to command-line-arguments
From: David Bremner The new argument handling is a bit more concise, and bit more flexible. It allows the input file name to go before the --accumulate option. --- notmuch-restore.c | 37 +++-- 1 files changed, 15 insertions(+), 22 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 13b4325..708f3e9 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -18,8 +18,6 @@ * Author: Carl Worth */ -#include - #include "notmuch-client.h" int @@ -29,12 +27,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_bool_t synchronize_flags; notmuch_bool_t accumulate = FALSE; +char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; size_t line_size; ssize_t line_len; regex_t regex; int rerr; +int opt_index; config = notmuch_config_open (ctx, NULL, NULL); if (config == NULL) @@ -47,37 +47,30 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); -struct option options[] = { - { "accumulate", no_argument, 0, 'a' }, - { 0, 0, 0, 0} +notmuch_opt_desc_t options[] = { + { "in-file", 'i', NOTMUCH_OPT_POSITION, 0, &input_file_name }, + { "accumulate", 'a', NOTMUCH_OPT_BOOLEAN, 0, &accumulate }, + { 0, 0, 0, 0, 0 } }; -int opt; -do { - opt = getopt_long (argc, argv, "", options, NULL); +opt_index = parse_arguments (argc, argv, options, 1); - switch (opt) { - case 'a': - accumulate = 1; - break; - case '?': - return 1; - break; - } - -} while (opt != -1); +if (opt_index < 0) { + /* diagnostics already printed */ + exit(1); +} -if (optind < argc) { - input = fopen (argv[optind], "r"); +if (input_file_name) { + input = fopen (input_file_name, "r"); if (input == NULL) { fprintf (stderr, "Error opening %s for reading: %s\n", -argv[optind], strerror (errno)); +input_file_name, strerror (errno)); return 1; } optind++; } -if (optind < argc) { +if (opt_index < argc) { fprintf (stderr, "Cannot read dump from more than one file: %s\n", argv[optind]); -- 1.7.7.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/4] notmuch-dump: convert to command-line-arguments
From: David Bremner The output file is handled via positional arguments. There are currently no "normal" options. --- notmuch-dump.c | 31 +++ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/notmuch-dump.c b/notmuch-dump.c index a490917..c568734 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -41,27 +41,34 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[]) if (notmuch == NULL) return 1; -argc--; argv++; /* skip subcommand argument */ +char *output_file_name = NULL; +int opt_index; -if (argc && strcmp (argv[0], "--") != 0) { +notmuch_opt_desc_t options[] = { + { "out-file", 'o', NOTMUCH_OPT_POSITION, 0, &output_file_name }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + /* diagnostics already printed */ + exit(1); +} + +if (output_file_name) { fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n"); - output = fopen (argv[0], "w"); + output = fopen (output_file_name, "w"); if (output == NULL) { fprintf (stderr, "Error opening %s for writing: %s\n", -argv[0], strerror (errno)); +output_file_name, strerror (errno)); return 1; } - argc--; - argv++; } -if (argc && strcmp (argv[0], "--") == 0){ - argc--; - argv++; -} -if (argc) { - query_str = query_string_from_args (notmuch, argc, argv); +if (opt_index < argc) { + query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index); if (query_str == NULL) { fprintf (stderr, "Out of memory.\n"); return 1; -- 1.7.7.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/4] command-line-arguments.[ch]: new argument parsing framework for notmuch.
From: David Bremner As we noticed when Jani kindly converted things to getopt_long, much of the work in argument parsing in notmuch is due to the the key-value style arguments like --format=(raw|json|text). The framework here provides positional arguments, simple switches, and --key=value style arguments that can take a value being an integer, a string, or one of a set of keywords. --- Makefile.local |1 + command-line-arguments.c | 159 ++ command-line-arguments.h | 39 +++ notmuch-client.h |1 + 4 files changed, 200 insertions(+), 0 deletions(-) create mode 100644 command-line-arguments.c create mode 100644 command-line-arguments.h diff --git a/Makefile.local b/Makefile.local index 15e6d88..28e371a 100644 --- a/Makefile.local +++ b/Makefile.local @@ -295,6 +295,7 @@ clean: distclean: clean notmuch_client_srcs = \ + command-line-arguments.c\ debugger.c \ gmime-filter-reply.c\ gmime-filter-headers.c \ diff --git a/command-line-arguments.c b/command-line-arguments.c new file mode 100644 index 000..96ed9ba --- /dev/null +++ b/command-line-arguments.c @@ -0,0 +1,159 @@ +#include +#include +#include +#include "error_util.h" +#include "command-line-arguments.h" + +/* + Search the array of keywords for a given argument, assigning the + output variable to the corresponding value. Return FALSE if nothing + matches. +*/ + +static notmuch_bool_t +_process_keyword_arg (const notmuch_opt_desc_t *arg_desc, const char *arg_str) { + +notmuch_keyword_t *keywords = arg_desc->keywords; + +while (keywords->name) { + if (strcmp (arg_str, keywords->name) == 0) { + if (arg_desc->output_var) { + *((int *)arg_desc->output_var) = keywords->value; + } + return TRUE; + } + keywords++; +} +fprintf (stderr, "unknown keyword: %s\n", arg_str); +return FALSE; +} + +/* + Search for the {pos_arg_index}th position argument, return FALSE if + that does not exist. +*/ + +notmuch_bool_t +parse_position_arg (const char *arg_str, int pos_arg_index, const notmuch_opt_desc_t *arg_desc) { + +int pos_arg_counter = 0; +while (arg_desc->name){ + if (arg_desc->opt_type == NOTMUCH_OPT_POSITION) { + if (pos_arg_counter == pos_arg_index) { + if (arg_desc->output_var) { + *((const char **)arg_desc->output_var) = arg_str; + } + return TRUE; + } + pos_arg_counter++; + } + arg_desc++; +} +return FALSE; +} + +notmuch_bool_t +parse_option (const char *arg, + const notmuch_opt_desc_t *options) { + +assert(arg); +assert(options); + +arg += 2; + +const notmuch_opt_desc_t *try = options; +while (try->name) { + if (strncmp (arg, try->name, strlen (try->name)) == 0) { + char next = arg[strlen (try->name)]; + const char *value= arg+strlen(try->name)+1; + + char *endptr; + + /* Everything but boolean arguments (switches) needs a +* delimiter, and a non-zero length value +*/ + + if (try->opt_type != NOTMUCH_OPT_BOOLEAN) { + if (next != '=' && next != ':') return FALSE; + if (value[0] == 0) return FALSE; + } else { + if (next != 0) return FALSE; + } + + if (try->output_var == NULL) + INTERNAL_ERROR ("output pointer NULL for option %s", try->name); + + switch (try->opt_type) { + case NOTMUCH_OPT_KEYWORD: + return _process_keyword_arg (try, value); + break; + case NOTMUCH_OPT_BOOLEAN: + *((notmuch_bool_t *)try->output_var) = TRUE; + return TRUE; + break; + case NOTMUCH_OPT_INT: + *((int *)try->output_var) = strtol (value, &endptr, 10); + return (*endptr == 0); + break; + case NOTMUCH_OPT_STRING: + *((const char **)try->output_var) = value; + case NOTMUCH_OPT_POSITION: + case NOTMUCH_OPT_NULL: + default: + INTERNAL_ERROR ("unknown or unhandled option type %d", try->opt_type); + /*UNREACHED*/ + } + } + try++; +} +fprintf (stderr, "Unrecognized option: --%s\n", arg); +return FALSE; +} + +/* + Parse command line arguments according to structure options, + starting at position opt_index. + + All output of parsed values is via pointers in options. + + Parsing stops at -- (consumed) or at the (k+1)st argument + not starting with -- (a "positional argument") if options contains + k positional argument descriptors. + + Returns the index of first non-parsed argument, or -1 in case of error. + + */ +int +parse_arguments (int argc, char **argv, +
[PATCH 4/4] notmuch-search: convert to command-line-arguments
From: David Bremner The switch on format_sel is slightly clunky, but it doesn't seem worth special casing argument processing for function pointers, when I think the function pointer approach will be modified/abandoned. --- notmuch-search.c | 109 - 1 files changed, 41 insertions(+), 68 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 36686d1..2ae7597 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -415,81 +415,54 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_str; -char *opt; notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST; const search_format_t *format = &format_text; -int i, ret; +int opt_index, ret; output_t output = OUTPUT_SUMMARY; int offset = 0; int limit = -1; /* unlimited */ -argc--; argv++; /* skip subcommand argument */ - -for (i = 0; i < argc && argv[i][0] == '-'; i++) { - if (strcmp (argv[i], "--") == 0) { - i++; - break; - } -if (STRNCMP_LITERAL (argv[i], "--sort=") == 0) { - opt = argv[i] + sizeof ("--sort=") - 1; - if (strcmp (opt, "oldest-first") == 0) { - sort = NOTMUCH_SORT_OLDEST_FIRST; - } else if (strcmp (opt, "newest-first") == 0) { - sort = NOTMUCH_SORT_NEWEST_FIRST; - } else { - fprintf (stderr, "Invalid value for --sort: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--offset=") == 0) { - char *p; - opt = argv[i] + sizeof ("--offset=") - 1; - offset = strtol (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --offset: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--limit=") == 0) { - char *p; - opt = argv[i] + sizeof ("--limit=") - 1; - limit = strtoul (opt, &p, 10); - if (*opt == '\0' || p == opt || *p != '\0') { - fprintf (stderr, "Invalid value for --limit: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { - opt = argv[i] + sizeof ("--format=") - 1; - if (strcmp (opt, "text") == 0) { - format = &format_text; - } else if (strcmp (opt, "json") == 0) { - format = &format_json; - } else { - fprintf (stderr, "Invalid value for --format: %s\n", opt); - return 1; - } - } else if (STRNCMP_LITERAL (argv[i], "--output=") == 0) { - opt = argv[i] + sizeof ("--output=") - 1; - if (strcmp (opt, "summary") == 0) { - output = OUTPUT_SUMMARY; - } else if (strcmp (opt, "threads") == 0) { - output = OUTPUT_THREADS; - } else if (strcmp (opt, "messages") == 0) { - output = OUTPUT_MESSAGES; - } else if (strcmp (opt, "files") == 0) { - output = OUTPUT_FILES; - } else if (strcmp (opt, "tags") == 0) { - output = OUTPUT_TAGS; - } else { - fprintf (stderr, "Invalid value for --output: %s\n", opt); - return 1; - } - } else { - fprintf (stderr, "Unrecognized option: %s\n", argv[i]); - return 1; - } +enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT } + format_sel = NOTMUCH_FORMAT_TEXT; + +notmuch_opt_desc_t options[] = { + { "sort", 's', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "oldest-first", NOTMUCH_SORT_OLDEST_FIRST }, + { "newest-first", NOTMUCH_SORT_NEWEST_FIRST }, + {0, 0} }, + &sort }, + { "format", 'f', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, + { "text", NOTMUCH_FORMAT_TEXT }, + {0, 0} }, + &format_sel }, + { "output", 'o', NOTMUCH_OPT_KEYWORD, + (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, + { "threads", OUTPUT_THREADS }, + { "messages", OUTPUT_MESSAGES }, + { "files", OUTPUT_FILES }, + { "tags", OUTPUT_TAGS }, + {0, 0} }, + &output }, + { "offset", 'O', NOTMUCH_OPT_INT, 0, &offset }, + { "limit", 'L', NOTMUCH_OPT_INT, 0, &limit }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); + +if (opt_index < 0) { + exit(1); } -argc -= i; -argv += i; +switch (format_sel) { +case NOTMUCH_FORMAT_TEXT: + format = &for
Re: [PATCH 1/4] notmuch-opts.[ch]: new argument parsing framework for notmuch.
On Tue, 06 Dec 2011 22:41:23 +0200, Jani Nikula wrote: > > We chatted about reserving notmuch-*.c to notmuch commands, but I guess > it was only after you sent these. I think it would make sense though. renamed to "command-line-arguments.[ch]". Hard luck for those of you on 8.3 file systems. > > +/* > > + Search the list of keywords for a given argument, assigning the > > + output variable to the corresponding value. Return FALSE if nothing > > + matches. > > +*/ > > Array of keywords to be really pedantic. Done. > > + > > +/* skip delimiter */ > > +arg_str++; > > I think the caller should check and skip the delimiters. See my comments > below where this gets called. done, and error checking tightened up. > > So if ->output_var is NULL, the parameter is accepted but silently > ignored? I'm not sure if I should consider this a feature or a bug. :) >From one extreme to another, it is now an INTERNAL_ERROR to have output_var NULL. I couldn't see a use case for silently ignoring command line arguments. > > +parse_option (const char *arg, > > + const notmuch_opt_desc_t *options){ > > Missing space between ) and {. done. but you missed some other missing spaces ;). > I think here you should check that arg[strlen(try->name)] is '=' or ':' > for NOTMUCH_OPT_KEYWORD and NOTMUCH_OPT_INT, and '\0' otherwise. After > the check, you could pass just the value part to _process_keyword_arg(). done. > I can't figure out if and how you handle arguments with arbitrary string > values (for example --file=/path/to/file). You do specify --in-file and > --out-file in later patches, but those are with NOTMUCH_OPT_POSITION, there is now NOTMUCH_OPT_STRING which does this, but it is untested as notmuch doesn't take these kind of arguments at the moment (restore --match is one example, but those patches are unmerged so far). > I'm not sure if much weight should be put to getopt_long() > compatibility, but it accepts "--parameter value" as well as > "--parameter=value". Yeah, maybe I shouldn't let the implementation drive things this much, but having one argmument per element of argv simplfies things nicely. For now, I will skip this. > > I think notmuch_parse_args() is a complicated function enough to warrant > a proper description here. Done. > > > +int > > +notmuch_parse_args (int argc, char **argv, const notmuch_opt_desc_t > > *options, int opt_index){ > > Missing space between ) and {. Done. > > +typedef struct notmuch_opt { > > +int arg_id; > > +int keyword_id; > > +const char *string; > > +} notmuch_opt_t; > > You're not using this for anything, are you? Oops, deleted. > > > + > > +notmuch_bool_t > > +parse_option (const char *arg, const notmuch_opt_desc_t* options); > > + > > +notmuch_bool_t > > +parse_position_arg (const char *arg, > > + int position_arg_index, > > + const notmuch_opt_desc_t* options); > > Is there a good reason the above are not static? I had in mind to provide the user with the option of a getopt style loop if the loop in parse_arguments was not flexible enough. I could leave them as static until such a need arises, I suppose. Thanks for the review! Updated series to follow. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
RFC: don't ask users to do 'sudo make install'
Hey everyone, I just noticed that running configure says: > All required packages were found. You may now run the following > commands to compile and install notmuch: > >make >sudo make install I think that this is a very poor advice in general and we shouldn't give it to users. I know that the default prefix is /usr/local, but installing notmuch this way makes it very hard to uninstall it later (there is no uninstall target). If someone decides to install notmuch from source and later on using his or her favorite package manager, the once installed notmuch in /usr/local will still be found and before the one in /usr and thus be preferred. Maybe we should mention stow (https://www.gnu.org/s/stow/)? Justus .signature Description: Binary data ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: add a function to run Python tests
The new test_python() function makes writing Python tests a little easier: - it sets the environment variables as needed - it redirects stdout to the OUTPUT file (like test_emacs()). This commit also declares python as an external prereq. The stdout redirection is required to avoid trouble when running commands like "python 'script' | sort > OUTPUT": in such a case, any error due to a missing external prereq would be "swallowed" by sort, resulting to a failed test instead of a skipped one. --- test/python |6 ++ test/test-lib.sh |9 + 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/test/python b/test/python index f737749..c3aa726 100755 --- a/test/python +++ b/test/python @@ -5,9 +5,7 @@ test_description="python bindings" add_email_corpus test_begin_subtest "compare thread ids" -LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib \ -PYTHONPATH=$TEST_DIRECTORY/../bindings/python \ -python < OUTPUT +test_python < EXPECTED -test_expect_equal_file OUTPUT EXPECTED +test_expect_equal_file <(sort OUTPUT) EXPECTED test_done diff --git a/test/test-lib.sh b/test/test-lib.sh index a975957..519bd84 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -919,6 +919,14 @@ test_emacs () { emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)" } +test_python() { + export LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib + export PYTHONPATH=$TEST_DIRECTORY/../bindings/python + + (echo "import sys; _orig_stdout=sys.stdout; sys.stdout=open('OUTPUT', 'w')"; cat) \ + | python - +} + test_reset_state_ () { test -z "$test_init_done_" && test_init_ @@ -1148,3 +1156,4 @@ test_declare_external_prereq emacs test_declare_external_prereq emacsclient test_declare_external_prereq gdb test_declare_external_prereq gpg +test_declare_external_prereq python -- 1.7.8 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] test: use python2 instead of python if available
Some distros (Arch Linux) ship Python as python2 and Python 3 as python. Checking for python2 is necessary for the Python tests to work on these platforms. --- test/test-lib.sh |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/test/test-lib.sh b/test/test-lib.sh index 519bd84..155ad3c 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -923,8 +923,14 @@ test_python() { export LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib export PYTHONPATH=$TEST_DIRECTORY/../bindings/python + # Some distros (e.g. Arch Linux) ship Python 2.* as /usr/bin/python2, + # most others as /usr/bin/python. So first try python2, and fallback to + # python if python2 doesn't exist. + cmd=python2 + [[ "$test_missing_external_prereq_python2_" = t ]] && cmd=python + (echo "import sys; _orig_stdout=sys.stdout; sys.stdout=open('OUTPUT', 'w')"; cat) \ - | python - + | $cmd - } test_reset_state_ () { @@ -1157,3 +1163,4 @@ test_declare_external_prereq emacsclient test_declare_external_prereq gdb test_declare_external_prereq gpg test_declare_external_prereq python +test_declare_external_prereq python2 -- 1.7.8 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
ANNOUNCE: nottoomuch-addresses.pl
On Tue, 06 Dec 2011 08:45:49 +1100, Bart Bunting wrote: > Hi, > > I agree with Jamie on this one. > > The case sensitivity appears to get in the way of searching. I also > think that enabling the regular expressionsearching is a good idea. > > All in all though this is great. I did version 1.2 of nottoomuch-addresses.sh which does case insensitive searches. Note the renaming to .sh -- it is now shell script wrapper which runs 'exec grep -aiF "$*" -- grep is locale-aware which makes case insensitivity work outside of ASCII range. The perl code to create/update addresses file is exactly the same as in version 1.1 So, the current version (1.2) is available at http://www.iki.fi/too/nottoomuch/nottoomuch-addresses.sh sha1sum is 03aa8bcf4e32d47e453fc081376843ef03a427ad and doc page is at http://www.iki.fi/too/nottoomuch/nottoomuch-addresses/ > The only other idea I have, which is only half formed, is that it would > be nice to prioritize emails that are more important than others. > > I'm not sure exactly how this would work but something like: > - Addresses I have actualy sent email to rather than jjust received from > get a high priority in the result. > - Addresses that have sent me email directly rather than just to a list > get next priority. > - All other emails after that. > > Also some sort of weighting within the groups to do with frequency of > emails sent or something. > > Does that make sense? Would it be hard to implement? Neat idea. To implement the feature into program is not too hard. Somehow determine weight for each email address, store that into hash (key address, value weight). Then, when writing addresses file, sort first by weight and then ascii order. The hash is also stored to disk (using tie()) and used when address file updated. Now, how to determine good (generic) rules for weights (and maybe some user-defined rules as well) is the question. > > Cheers > > Bart Thanks, Tomi