[PATCH v2 1/3] mime node: Record depth-first part numbers
On Sun, 22 Jan 2012 21:31:11 -0500, Austin Clements wrote: > This makes the part numbers readily accessible to formatters. > Hierarchical part numbering would be a more natural and efficient fit > for MIME and may be the way to go in the future, but depth-first > numbering maintains compatibility with what we currently do. > --- LGTM. I did not understand the logic so after a while I concentrated on code robustness (i.e. this doesn't break anything). Future work depends on this. Tomi
[PATCH v2 3/3] show: Introduce mime_node formatter callback
On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements wrote: > This callback is the gateway to the new mime_node_t-based formatters. > This maintains backwards compatibility so the formatters can be > transitioned one at a time. Once all formatters are converted, the > formatter structure can be reduced to only message_set_{start,sep,end} > and part, most of show_message can be deleted, and all of > show-message.c can be deleted. > --- LGTM. Tomi
[PATCH 6/6] emacs: modify the default show-mode key bindings for archiving
This is also +1 for me. /Xavier
[RFC PATCH 3/4] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag
Quoth Mark Walters on Jan 24 at 1:18 am: > Add the actual NOTMUCH_MESSAGE_FLAG_EXCLUDED flag. > > --- > lib/notmuch-private.h |1 + > lib/notmuch.h |3 ++- > lib/query.cc | 11 +++ > lib/thread.cc | 14 ++ > 4 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h > index e791bb0..cb3eca6 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -216,6 +216,7 @@ _notmuch_thread_create (void *ctx, > notmuch_database_t *notmuch, > unsigned int seed_doc_id, > notmuch_doc_id_set_t *match_set, > + notmuch_doc_id_set_t *excluded_doc_ids, > notmuch_sort_t sort); > > /* message.cc */ > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 7929fe7..cf0d45d 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -895,7 +895,8 @@ notmuch_message_get_filenames (notmuch_message_t > *message); > > /* Message flags */ > typedef enum _notmuch_message_flag { > -NOTMUCH_MESSAGE_FLAG_MATCH > +NOTMUCH_MESSAGE_FLAG_MATCH, > +NOTMUCH_MESSAGE_FLAG_EXCLUDED > } notmuch_message_flag_t; > > /* Get a value of a flag for the email corresponding to 'message'. */ > diff --git a/lib/query.cc b/lib/query.cc > index 92fa834..69e32bd 100644 > --- a/lib/query.cc > +++ b/lib/query.cc > @@ -55,6 +55,7 @@ struct visible _notmuch_threads { > /* The set of matched docid's that have not been assigned to a > * thread. Initially, this contains every docid in doc_ids. */ > notmuch_doc_id_set_t match_set; > +notmuch_doc_id_set_t *excluded_doc_ids; > }; > > static notmuch_bool_t > @@ -302,6 +303,9 @@ _notmuch_mset_messages_get (notmuch_messages_t *messages) > INTERNAL_ERROR ("a messages iterator contains a non-existent document > ID.\n"); > } > > +if (_notmuch_doc_id_set_contains (messages->excluded_doc_ids, doc_id)) > + notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, TRUE); > + > return message; > } > > @@ -314,10 +318,6 @@ _notmuch_mset_messages_move_to_next (notmuch_messages_t > *messages) > > mset_messages->iterator++; > > -while ((mset_messages->iterator != mset_messages->iterator_end) && > -(_notmuch_doc_id_set_contains (messages->excluded_doc_ids, > - *mset_messages->iterator))) > - mset_messages->iterator++; > } > > static notmuch_bool_t > @@ -403,6 +403,8 @@ notmuch_query_search_threads (notmuch_query_t *query) > notmuch_messages_move_to_next (messages); > } > threads->doc_id_pos = 0; > +/* the excluded messages are in query context so this should be ok */ > +threads->excluded_doc_ids = messages->excluded_doc_ids; > > talloc_free (messages); > > @@ -452,6 +454,7 @@ notmuch_threads_get (notmuch_threads_t *threads) > threads->query->notmuch, > doc_id, > &threads->match_set, > +threads->excluded_doc_ids, > threads->query->sort); > } > > diff --git a/lib/thread.cc b/lib/thread.cc > index 0435ee6..6ea2a44 100644 > --- a/lib/thread.cc > +++ b/lib/thread.cc > @@ -302,7 +302,8 @@ _thread_set_subject_from_message (notmuch_thread_t > *thread, > static void > _thread_add_matched_message (notmuch_thread_t *thread, >notmuch_message_t *message, > - notmuch_sort_t sort) > + notmuch_sort_t sort, > + notmuch_bool_t excluded) > { > time_t date; > notmuch_message_t *hashed_message; > @@ -321,7 +322,8 @@ _thread_add_matched_message (notmuch_thread_t *thread, > _thread_set_subject_from_message (thread, message); > } > > -thread->matched_messages++; > +if (!excluded) > + thread->matched_messages++; I interpret notmuch_thread_get_matched_messages as returning the number of messages with the "match" flag, which this approach changes. I would suggest introducing a new, more flexible API along the lines of /* Return the number of messages in thread for which * notmuch_message_get_flag(msg) & msg == match. */ int notmuch_thread_count_flags (notmuch_thread_t *thread, int mask, int match); This would subsume the existing notmuch_thread_get_{matched,total}_messages APIs. It could either iterate over the message list (which, curiously, we don't currently track), or it could proactively count messages in an array indexed by the message's flags (which wouldn't scale to large numbers of flags but, for now, would only be of length 4). > > if (g_hash_table_lookup_extended (thread->message_hash, > notmuch_message_get_message_id (message), NULL, > @@ -392,6 +394,7 @@ _notmuch_thread_create (void *ctx, >
[RFC PATCH 2/4] Add NOTMUCH_MESSAGE_FLAG_EXCLUDED flag
The overall structure of this series looks great. There's obviously a lot of clean up to do, but I'll reply with a few high-level comments. Quoth Mark Walters on Jan 24 at 1:18 am: > Form excluded doc_ids set and use that to exclude messages. > Should be no functional change. > > --- > lib/notmuch-private.h |1 + > lib/query.cc | 28 ++-- > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h > index 7bf153e..e791bb0 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -401,6 +401,7 @@ typedef struct _notmuch_message_list { > */ > struct visible _notmuch_messages { > notmuch_bool_t is_of_list_type; > +notmuch_doc_id_set_t *excluded_doc_ids; > notmuch_message_node_t *iterator; > }; > > diff --git a/lib/query.cc b/lib/query.cc > index c25b301..92fa834 100644 > --- a/lib/query.cc > +++ b/lib/query.cc > @@ -57,6 +57,11 @@ struct visible _notmuch_threads { > notmuch_doc_id_set_t match_set; > }; > > +static notmuch_bool_t > +_notmuch_doc_id_set_init (void *ctx, > + notmuch_doc_id_set_t *doc_ids, > + GArray *arr); > + > notmuch_query_t * > notmuch_query_create (notmuch_database_t *notmuch, > const char *query_string) > @@ -173,6 +178,7 @@ notmuch_query_search_messages (notmuch_query_t *query) > "mail")); > Xapian::Query string_query, final_query, exclude_query; > Xapian::MSet mset; > + Xapian::MSetIterator iterator; > unsigned int flags = (Xapian::QueryParser::FLAG_BOOLEAN | > Xapian::QueryParser::FLAG_PHRASE | > Xapian::QueryParser::FLAG_LOVEHATE | > @@ -193,8 +199,21 @@ notmuch_query_search_messages (notmuch_query_t *query) > > exclude_query = _notmuch_exclude_tags (query, final_query); > > - final_query = Xapian::Query (Xapian::Query::OP_AND_NOT, > - final_query, exclude_query); > + enquire.set_weighting_scheme (Xapian::BoolWeight()); > + enquire.set_query (exclude_query); > + > + mset = enquire.get_mset (0, notmuch->xapian_db->get_doccount ()); > + > + GArray *excluded_doc_ids = g_array_new (FALSE, FALSE, sizeof (unsigned > int)); > + > + for (iterator = mset.begin (); iterator != mset.end (); iterator++) > + { > + unsigned int doc_id = *iterator; > + g_array_append_val (excluded_doc_ids, doc_id); > + } > + messages->base.excluded_doc_ids = talloc (query, _notmuch_doc_id_set); > + _notmuch_doc_id_set_init (query, messages->base.excluded_doc_ids, > + excluded_doc_ids); This might be inefficient for message-only queries, since it will fetch *all* excluded docids. This highlights a basic difference between message and thread search: thread search can return messages that don't match the original query and hence needs to know all potentially excluded messages, while message search can only return messages that match the original query. It's entirely possible this doesn't matter because Xapian probably still needs to fetch the full posting lists of the excluded terms, but it would be worth doing a quick/hacky benchmark to verify this, with enough excluded messages to make the cost non-trivial. If it does matter, you could pass in a flag indicating if the exclude query should be limited by the original query or not. Or you could do the limited exclude query in notmuch_query_search_messages and a separate open-ended exclude query in notmuch_query_search_threads. > > enquire.set_weighting_scheme (Xapian::BoolWeight()); > > @@ -294,6 +313,11 @@ _notmuch_mset_messages_move_to_next (notmuch_messages_t > *messages) > mset_messages = (notmuch_mset_messages_t *) messages; > > mset_messages->iterator++; > + > +while ((mset_messages->iterator != mset_messages->iterator_end) && > +(_notmuch_doc_id_set_contains (messages->excluded_doc_ids, > + *mset_messages->iterator))) > + mset_messages->iterator++; This seemed a little weird, since you remove it in the next patch. Is this just to keep the tests happy? (If so, it would be worth mentioning in the commit message; other reviewers will definitely have the same question.) > } > > static notmuch_bool_t
[PATCH 3/3] test: Add address cleaning tests.
On Thu, 19 Jan 2012 12:54:03 +, David Edmondson wrote: > Including some more test framework in test-lib.el. IMO test-lib.el changes should be in a separate patch. > --- > test/emacs-address-cleaning.el | 29 + > test/emacs-address-cleaning.sh | 12 > test/notmuch-test |1 + > test/test-lib.el | 30 ++ > 4 files changed, 72 insertions(+), 0 deletions(-) > create mode 100644 test/emacs-address-cleaning.el > create mode 100755 test/emacs-address-cleaning.sh > > diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el > new file mode 100644 > index 000..59e8d92 > --- /dev/null > +++ b/test/emacs-address-cleaning.el > @@ -0,0 +1,29 @@ > +(defun notmuch-test-address-cleaning-1 () > + (notmuch-test-compare (notmuch-show-clean-address "dme at dme.org") > + "dme at dme.org")) > + > +(defun notmuch-test-address-cleaning-2 () > + (let* ((input '("foo at bar.com" > + "" > + "Foo Bar " > + "foo at bar.com " > + "\"Foo Bar\" ")) > + (expected '("foo at bar.com" > + "foo at bar.com" > + "Foo Bar " > + "foo at bar.com" > + "Foo Bar ")) > + (output (mapcar #'notmuch-show-clean-address input))) > +(notmuch-test-compare output expected))) > + > +(defun notmuch-test-address-cleaning-3 () > + (let* ((input '("?? " > + "foo (at home) " > + "foo [at home] " > + "Foo Bar")) > + (expected '("?? " > + "foo (at home) " > + "foo [at home] " > + "Foo Bar")) > + (output (mapcar #'notmuch-show-clean-address input))) > +(notmuch-test-compare output expected))) At a glance, the tests look good. Though more review of the tests itself would be nice. > diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh > new file mode 100755 > index 000..1a6eff5 > --- /dev/null > +++ b/test/emacs-address-cleaning.sh > @@ -0,0 +1,12 @@ > +#!/usr/bin/env bash > + > +test_description="emacs address cleaning" > +. test-lib.sh > + > +for i in 1 2 3; do > +test_begin_subtest "notmuch-test-address-clean-$i" > +test_emacs_expect_t \ > + '(load "emacs-address-cleaning.el") > (notmuch-test-address-cleaning-'$i')' > +done > + > +test_done > diff --git a/test/notmuch-test b/test/notmuch-test > index d034f99..3f1740c 100755 > --- a/test/notmuch-test > +++ b/test/notmuch-test > @@ -53,6 +53,7 @@ TESTS=" >hooks >argument-parsing >emacs-test-functions.sh > + emacs-address-cleaning.sh > " > TESTS=${NOTMUCH_TESTS:=$TESTS} > > diff --git a/test/test-lib.el b/test/test-lib.el > index 1d51b8d..033270d 100644 > --- a/test/test-lib.el > +++ b/test/test-lib.el > @@ -20,6 +20,8 @@ > ;; > ;; Authors: Dmitry Kurochkin > > +(require 'cl);; This code is generally used uncompiled. > + > ;; `read-file-name' by default uses `completing-read' function to read > ;; user input. It does not respect `standard-input' variable which we > ;; use in tests to provide user input. So replace it with a plain > @@ -77,6 +79,7 @@ nothing." > (add-hook-counter 'notmuch-hello-mode-hook) > (add-hook-counter 'notmuch-hello-refresh-hook) > > + Please revert. > (defmacro notmuch-test-run-test (&rest body) >"Evaluate a BODY of test expressions and output the result." >`(with-temp-buffer > @@ -85,3 +88,30 @@ nothing." > result >(prin1-to-string result))) > (test-output > + > +;; Functions to help when writing tests: > + IMO this comment is misplaced. There are other functions above which also help to write tests e.g. `test-output. I think we should either remove the comment or move all test helpers in one place below the comment (in a separate patch). > +(defun notmuch-test-reporter (output expected) Please consider renaming to `notmuch-test-report-unexpected'. > + "Report that the `output' does not match the `expected' result." > + (concat "Expect:\t" (prin1-to-string expected) "\n" > + "Output:\t" (prin1-to-string output) "\n")) > + > +(defun notmuch-test-unsimilar (output expected) Please consider renaming to notmuch-test-unexpected. > + "`output' and `expected' are dissimilar. Show a summary of Previous line says "unsimilar", now it is "dissimilar". I am not sure which one is correct. Also, it is not clear what "similar" means for test results. IMO "not equal" would be more clear. > +the differences, ignoring similarities." > + (cond ((and (listp output) > + (listp expected)) > + (apply #'concat (loop for o in output > +for e in expected > +if (not (equal o e)) > +collect (notmuch-test-reporter o e > + > + (t > + ;; Catch all case. > +
[PATCH 2/3] test: Add `test_emacs_expect_t'.
On Thu, 19 Jan 2012 12:54:02 +, David Edmondson wrote: > Add a new test function to allow simpler testing of emacs > functionality. > > `test_emacs_expect_t' takes one argument - a list expression to > evaluate. The test passes if the expression returns `t', otherwise it > fails and the output is reported to the tester. > --- > > In the interest of moving forward, don't use eval. > > test/README |8 > test/emacs-test-functions.sh |9 + > test/notmuch-test|1 + > test/test-lib.el |9 + > test/test-lib.sh | 26 ++ > 5 files changed, 53 insertions(+), 0 deletions(-) > create mode 100755 test/emacs-test-functions.sh > > diff --git a/test/README b/test/README > index bde6db0..9dbe2ee 100644 > --- a/test/README > +++ b/test/README > @@ -189,6 +189,14 @@ library for your script to use. > tests that may run in the same Emacs instance. Use `let' instead > so the scope of the changed variables is limited to a single test. > > + test_emacs_expect_t > + > + This function executes the provided emacs lisp script within > + emacs in a manner similar to 'test_emacs'. The expressions should > + return the value `t' to indicate that the test has passed. If the > + test does not return `t' then it is considered failed and all data > + returned by the test is reported to the tester. > + > test_done > > Your test script must have test_done at the end. Its purpose > diff --git a/test/emacs-test-functions.sh b/test/emacs-test-functions.sh > new file mode 100755 > index 000..0e1f9fc > --- /dev/null > +++ b/test/emacs-test-functions.sh > @@ -0,0 +1,9 @@ > +#!/usr/bin/env bash > + > +test_description="emacs test function sanity" > +. test-lib.sh > + > +test_begin_subtest "emacs test function sanity" > +test_emacs_expect_t 't' > + > +test_done > diff --git a/test/notmuch-test b/test/notmuch-test > index 6a99ae3..d034f99 100755 > --- a/test/notmuch-test > +++ b/test/notmuch-test > @@ -52,6 +52,7 @@ TESTS=" >python >hooks >argument-parsing > + emacs-test-functions.sh > " > TESTS=${NOTMUCH_TESTS:=$TESTS} > > diff --git a/test/test-lib.el b/test/test-lib.el > index 3b817c3..1d51b8d 100644 > --- a/test/test-lib.el > +++ b/test/test-lib.el > @@ -76,3 +76,12 @@ nothing." > > (add-hook-counter 'notmuch-hello-mode-hook) > (add-hook-counter 'notmuch-hello-refresh-hook) > + > +(defmacro notmuch-test-run-test (&rest body) Please consider renaming to `notmuch-hello-run'. > + "Evaluate a BODY of test expressions and output the result." > + `(with-temp-buffer > + (let ((result (progn , at body))) > + (insert (if (stringp result) > +result > + (prin1-to-string result))) > + (test-output > diff --git a/test/test-lib.sh b/test/test-lib.sh > index 7c9ce24..9cf1b92 100644 > --- a/test/test-lib.sh > +++ b/test/test-lib.sh > @@ -503,6 +503,32 @@ test_expect_equal_file () > fi > } > > +test_emacs_expect_t () { > + test "$#" = 1 || error "bug in the test script: not 1 parameter to > test_emacs_expect_t" > + > + # Run the test. > + if ! test_skip "$test_subtest_name" > + then > + test_emacs "(notmuch-test-run-test $1)" >/dev/null > + fi > + > + # Restore state after the test. > + exec 1>&6 2>&7 # Restore stdout and stderr > + inside_subtest= > + > + # Report success/failure. > + if ! test_skip "$test_subtest_name" > + then > + result=$(cat OUTPUT) > + if [ "$result" = t ] > + then > + test_ok_ "$test_subtest_name" > + else > + test_failure_ "$test_subtest_name" "${result}" > + fi > + fi > +} Test_skip function must be called once during the test. Otherwise $test_count is incremented twice, perhaps there are other side effects as well. I think test_emacs_expect_t function must support optional $prereq argument like other test_equal_* functions (e.g. test_expect_equal). After fixing the above comments, test_emacs_expect_t function should look more like test_expect_equal and friends: exec 1>&6 2>&7# Restore stdout and stderr [4 lines almost identical to test_expect_equal] if ! test_skip "$test_subtest_name" test_emacs ... ... fi Regards, Dmitry > + > NOTMUCH_NEW () > { > notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found > [0-9]* total file' > -- > 1.7.8.3 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] test: Don't return the result of checking for running emacs to the tester.
LGTM. Regards, Dmitry
[PATCH] Automatically exclude tags in notmuch-show
Quoth Mark Walters on Jan 24 at 1:05 am: > > On Sun, 22 Jan 2012 20:52:22 -0500, Austin Clements > wrote: > > Quoth Mark Walters on Jan 23 at 1:13 am: > > > On Sun, 22 Jan 2012 13:16:09 -0500, Austin Clements > > > wrote: > > > > Quoth myself on Jan 20 at 12:18 pm: > > > > > Quoth Mark Walters on Jan 20 at 12:10 am: > > > > > > > > > > > > Ok Having said this is trivial I have found a problem. What should > > > > > > notmuch do if you do something like > > > > > > > > > > > > notmuch show id: > > > > > > and that message is marked with a deleted tag? To be consistent > > > > > > with the > > > > > > other cases (where a deleted message is in a matched thread) we > > > > > > might > > > > > > want to return the message with the not-matched flag set (eg in > > > > > > JSON). But my patch doesn't, as it never even sees the thread since > > > > > > it > > > > > > doesn't match. > > > > > > > > > > > > Looking at notmuch-show.c I think we should not apply the exclude > > > > > > tags > > > > > > to do_show_single, but usually should apply it to do_show. One > > > > > > solution > > > > > > which is simple and is at least close to right would be to get > > > > > > do_show > > > > > > to return the number of threads found. If this is zero then retry > > > > > > the > > > > > > query without the excludes (possible setting the match_flag to zero > > > > > > on > > > > > > each message since we know it does not match) > > > > > > > > > > > > This is not a completely correct solution as if you ask > > > > > > notmuch-show to > > > > > > show more than one thread it might threads which only contain > > > > > > deleted > > > > > > messages. > > > > > > > > > > > > I can't see other good possibilities without slowing down the normal > > > > > > path a lot (eg find all threads that match the original query and > > > > > > then > > > > > > apply the argument above). > > > > > > > > > > > > Any thoughts? > > > > > > > > > > Oh dear. > > > > > > > > > > Well, here's one idea. Instead of doing a single thread query in > > > > > show, do a thread query without the exclusions and then a message > > > > > query with the exclusions. Output all of the messages from the first > > > > > query, but use the results of the second query to determine which > > > > > messages are "matched". The same could be accomplished in the library > > > > > somewhat more efficiently, but it's not obvious to me what the API > > > > > would be. > > > > > > > > Here's a slightly crazier idea that's more library-invasive than the > > > > original approach, but probably better in the long run. > > > > > > > > Have notmuch_query_search_* return everything and make exclusion a > > > > message flag like NOTMUCH_MESSAGE_FLAG_MATCH. Tweak the definition of > > > > "matched" to mean "matched and not excluded" (specifically, a message > > > > would have the match flag or the excluded flag or neither, but not > > > > both). Search would skip threads with zero matched messages and I > > > > think show would Just Work. > > > > > > > > I can think of two ways to implement this. notmuch_query_search_* > > > > could perform both the original query and the query with exclusions > > > > and use the docid set from the second to compute the "excluded" > > > > message flag. Alternatively, it could examine the tags of each > > > > message directly to compute the flag. The latter is probably easier > > > > to implement, but probably slower. > > > > > > > > Thoughts? > > > > > > I have now thought about this some more and think I understand your idea > > > (and how it would work) rather better now. > > > > > > I would suggest one small change: the flags for the messages returned > > > should be "independent": so a message can match the query or not, and it > > > can be excluded or not, with all 4 combinations being possible. (The > > > consumer of notmuch_query_search_* would extract the information it > > > wanted.) > > > > I'd initially approached it this way, but went with redefining a > > "matched" messages because it had much less impact on the API. For > > example, with the redefined "match", > > notmuch_thread_get_matched_messages still does the right thing for > > search and things like the thread subject can still be based on > > "matched" messages. If we orthongonalize these flags, then we at > > least need to count matched non-excluded messages and provide an API > > to access this (while I don't have a solid argument against such an > > API it just seems weirdly specific to me). > > Ok I have an initial implementation of this which I will post as a reply > to this thread: it does make the flags orthogonal but that would be easy > to change. If we do want to keep match to mean match and not excluded > then I would argue for a third flag so that the emacs frontend could see > all 4 possibilities. Note that in your suggestion we still need to do > something in notmuch_thread_get_matched_messages to set the subject etc >
[PATCH 10/10] test: use emacsclient(1) for Emacs tests
On Tue, 28 Jun 2011 08:45:11 +0400, Dmitry Kurochkin wrote: > Before the change, every Emacs test ran in a separate Emacs > instance. Starting Emacs many times wastes considerable time and > it gets worse as the test suite grows. The patch solves this by > using a single Emacs server and emacsclient(1) to run multiple > tests. Emacs server is started on the first test_emacs call and > stopped when test_done is called. We take care not to leave > orphan Emacs processes behind when test is terminated by whatever > reason: Emacs server runs a watchdog that periodically checks > that the test is still running. > > Some tests need to provide user input. Before the change, this > was done using echo(1) to Emacs stdin. This no longer works and > instead `standard-input' variable is set accordingly to make > `read' return the appropriate string. I really like this too. +1 /Xavier
[PATCH v2 1/3] mime node: Record depth-first part numbers
On Sun, 22 Jan 2012 21:31:11 -0500, Austin Clements wrote: > This makes the part numbers readily accessible to formatters. > Hierarchical part numbering would be a more natural and efficient fit > for MIME and may be the way to go in the future, but depth-first > numbering maintains compatibility with what we currently do. > --- > mime-node.c | 37 ++--- > notmuch-client.h | 14 +- > 2 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/mime-node.c b/mime-node.c > index 27077f7..025c537 100644 > --- a/mime-node.c > +++ b/mime-node.c > @@ -112,6 +112,10 @@ mime_node_open (const void *ctx, notmuch_message_t > *message, > root->nchildren = 1; > root->ctx = mctx; > > +root->part_num = 0; > +root->next_child = 0; > +root->next_part_num = 1; > + > *root_out = root; > return NOTMUCH_STATUS_SUCCESS; > > @@ -137,7 +141,7 @@ _signature_validity_free (GMimeSignatureValidity **proxy) > #endif > > static mime_node_t * > -_mime_node_create (const mime_node_t *parent, GMimeObject *part) > +_mime_node_create (mime_node_t *parent, GMimeObject *part) > { > mime_node_t *node = talloc_zero (parent, mime_node_t); > GError *err = NULL; > @@ -150,6 +154,8 @@ _mime_node_create (const mime_node_t *parent, GMimeObject > *part) > talloc_free (node); > return NULL; > } > +node->parent = parent; > +node->part_num = node->next_part_num = -1; > > /* Deal with the different types of parts */ > if (GMIME_IS_PART (part)) { > @@ -267,9 +273,10 @@ _mime_node_create (const mime_node_t *parent, > GMimeObject *part) > } > > mime_node_t * > -mime_node_child (const mime_node_t *parent, int child) > +mime_node_child (mime_node_t *parent, int child) > { > GMimeObject *sub; > +mime_node_t *node; > > if (!parent || child < 0 || child >= parent->nchildren) > return NULL; > @@ -287,7 +294,31 @@ mime_node_child (const mime_node_t *parent, int child) > INTERNAL_ERROR ("Unexpected GMimeObject type: %s", > g_type_name (G_OBJECT_TYPE (parent->part))); > } > -return _mime_node_create (parent, sub); > +node = _mime_node_create (parent, sub); > + > +if (child == parent->next_child && parent->next_part_num != -1) { > + /* We're traversing in depth-first order. Record the child's > + * depth-first numbering. */ > + node->part_num = parent->next_part_num; > + node->next_part_num = node->part_num + 1; > + > + /* Drop the const qualifier because these are internal fields > + * whose mutability doesn't affect the interface. */ Leftover comment from the old version. Otherwise, the code looks good, with the disclaimer that I don't know much about MIME or gmime. BR, Jani. > + parent->next_child++; > + parent->next_part_num = -1; > + > + if (node->nchildren == 0) { > + /* We've reached a leaf, so find the parent that has more > + * children and set it up to number its next child. */ > + mime_node_t *it = node; > + while (it && it->next_child == it->nchildren) > + it = it->parent; > + if (it) > + it->next_part_num = node->part_num + 1; > + } > +} > + > +return node; > } > > static mime_node_t * > diff --git a/notmuch-client.h b/notmuch-client.h > index 9c1d383..abfe5d3 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -297,6 +297,13 @@ typedef struct mime_node { > /* The number of children of this part. */ > int nchildren; > > +/* The parent of this node or NULL if this is the root node. */ > +struct mime_node *parent; > + > +/* The depth-first part number of this child if the MIME tree is > + * being traversed in depth-first order, or -1 otherwise. */ > +int part_num; > + > /* True if decryption of this part was attempted. */ > notmuch_bool_t decrypt_attempted; > /* True if decryption of this part's child succeeded. In this > @@ -324,6 +331,11 @@ typedef struct mime_node { > /* Internal: For successfully decrypted multipart parts, the > * decrypted part to substitute for the second child. */ > GMimeObject *decrypted_child; > + > +/* Internal: The next child for depth-first traversal and the part > + * number to assign it (or -1 if unknown). */ > +int next_child; > +int next_part_num; > } mime_node_t; > > /* Construct a new MIME node pointing to the root message part of > @@ -356,7 +368,7 @@ mime_node_open (const void *ctx, notmuch_message_t > *message, > * an error message on stderr). > */ > mime_node_t * > -mime_node_child (const mime_node_t *parent, int child); > +mime_node_child (mime_node_t *parent, int child); > > /* Return the nth child of node in a depth-first traversal. If n is > * 0, returns node itself. Returns NULL if there is no such part. */ > -- > 1.7.7.3 > > __
[PATCH 08/10] test: set variables using `let' instead of `setq' in Emacs tests
On Tue, 28 Jun 2011 08:45:09 +0400, Dmitry Kurochkin wrote: > Using `setq' for setting variables in Emacs tests affect other > tests that may run in the same Emacs environment. Currently it > works because each test is run in a separate Emacs instance. But > in the future multiple tests will run in a single Emacs instance. > The patch changes all variables to use `let', so the scope of the > change is limited to a single test. +1 (definetely) /Xavier
[PATCH v2 1/6] search: rename auto_exclude_tags to {search, }exclude_tags
On Mon, 23 Jan 2012 05:22:32 +0100, Pieter Praet wrote: > All other config-related functions and args include the section > title in their name, so for the sake of consistency, mirror that. > All six pushed, d
[PATCH v5 2/2] show: Introduce mime_node formatter callback
This callback is the gateway to the new mime_node_t-based formatters. This maintains backwards compatibility so the formatters can be transitioned one at a time. Once all formatters are converted, the formatter structure can be reduced to only message_set_{start,sep,end} and part, most of show_message can be deleted, and all of show-message.c can be deleted. --- notmuch-client.h |6 ++ notmuch-reply.c |2 +- notmuch-show.c | 21 + 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/notmuch-client.h b/notmuch-client.h index abfe5d3..093c789 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -62,8 +62,14 @@ #define STRINGIFY(s) STRINGIFY_(s) #define STRINGIFY_(s) #s +struct mime_node; +struct notmuch_show_params; + typedef struct notmuch_show_format { const char *message_set_start; +void (*part) (const void *ctx, + struct mime_node *node, int indent, + const struct notmuch_show_params *params); const char *message_start; void (*message) (const void *ctx, notmuch_message_t *message, diff --git a/notmuch-reply.c b/notmuch-reply.c index bf67960..f55b1d2 100644 --- a/notmuch-reply.c +++ b/notmuch-reply.c @@ -31,7 +31,7 @@ static void reply_part_content (GMimeObject *part); static const notmuch_show_format_t format_reply = { -"", +"", NULL, "", NULL, "", NULL, reply_headers_message_part, ">\n", "", diff --git a/notmuch-show.c b/notmuch-show.c index 682aa71..dec799c 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -42,7 +42,7 @@ static void format_part_end_text (GMimeObject *part); static const notmuch_show_format_t format_text = { -"", +"", NULL, "\fmessage{ ", format_message_text, "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n", "\fbody{\n", @@ -89,7 +89,7 @@ static void format_part_end_json (GMimeObject *part); static const notmuch_show_format_t format_json = { -"[", +"[", NULL, "{", format_message_json, "\"headers\": {", format_headers_json, format_headers_message_part_json, "}", ", \"body\": [", @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx, unused (int indent)); static const notmuch_show_format_t format_mbox = { -"", +"", NULL, "", format_message_mbox, "", NULL, NULL, "", "", @@ -129,7 +129,7 @@ static void format_part_content_raw (GMimeObject *part); static const notmuch_show_format_t format_raw = { -"", +"", NULL, "", NULL, "", NULL, format_headers_message_part_text, "\n", "", @@ -850,6 +850,19 @@ show_message (void *ctx, int indent, notmuch_show_params_t *params) { +if (format->part) { + void *local = talloc_new (ctx); + mime_node_t *root, *part; + + if (mime_node_open (local, message, params->cryptoctx, params->decrypt, + &root) == NOTMUCH_STATUS_SUCCESS && + (part = mime_node_seek_dfs (root, (params->part < 0 ? + 0 : params->part + format->part (local, part, indent, params); + talloc_free (local); + return; +} + if (params->part <= 0) { fputs (format->message_start, stdout); if (format->message) -- 1.7.7.3
[PATCH v5 1/2] mime node: Record depth-first part numbers
This makes the part numbers readily accessible to formatters. Hierarchical part numbering would be a more natural and efficient fit for MIME and may be the way to go in the future, but depth-first numbering maintains compatibility with what we currently do. --- mime-node.c | 38 +++--- notmuch-client.h | 14 +- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/mime-node.c b/mime-node.c index 27077f7..d6b4506 100644 --- a/mime-node.c +++ b/mime-node.c @@ -112,6 +112,11 @@ mime_node_open (const void *ctx, notmuch_message_t *message, root->nchildren = 1; root->ctx = mctx; +root->parent = NULL; +root->part_num = 0; +root->next_child = 0; +root->next_part_num = 1; + *root_out = root; return NOTMUCH_STATUS_SUCCESS; @@ -137,7 +142,7 @@ _signature_validity_free (GMimeSignatureValidity **proxy) #endif static mime_node_t * -_mime_node_create (const mime_node_t *parent, GMimeObject *part) +_mime_node_create (mime_node_t *parent, GMimeObject *part) { mime_node_t *node = talloc_zero (parent, mime_node_t); GError *err = NULL; @@ -150,6 +155,9 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) talloc_free (node); return NULL; } +node->parent = parent; +node->part_num = node->next_part_num = -1; +node->next_child = 0; /* Deal with the different types of parts */ if (GMIME_IS_PART (part)) { @@ -267,9 +275,10 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) } mime_node_t * -mime_node_child (const mime_node_t *parent, int child) +mime_node_child (mime_node_t *parent, int child) { GMimeObject *sub; +mime_node_t *node; if (!parent || child < 0 || child >= parent->nchildren) return NULL; @@ -287,7 +296,30 @@ mime_node_child (const mime_node_t *parent, int child) INTERNAL_ERROR ("Unexpected GMimeObject type: %s", g_type_name (G_OBJECT_TYPE (parent->part))); } -return _mime_node_create (parent, sub); +node = _mime_node_create (parent, sub); + +if (child == parent->next_child && parent->next_part_num != -1) { + /* We're traversing in depth-first order. Record the child's +* depth-first numbering. */ + node->part_num = parent->next_part_num; + node->next_part_num = node->part_num + 1; + + /* Prepare the parent for its next depth-first child. */ + parent->next_child++; + parent->next_part_num = -1; + + if (node->nchildren == 0) { + /* We've reached a leaf, so find the parent that has more +* children and set it up to number its next child. */ + mime_node_t *iter = node->parent; + while (iter && iter->next_child == iter->nchildren) + iter = iter->parent; + if (iter) + iter->next_part_num = node->part_num + 1; + } +} + +return node; } static mime_node_t * diff --git a/notmuch-client.h b/notmuch-client.h index 9c1d383..abfe5d3 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -297,6 +297,13 @@ typedef struct mime_node { /* The number of children of this part. */ int nchildren; +/* The parent of this node or NULL if this is the root node. */ +struct mime_node *parent; + +/* The depth-first part number of this child if the MIME tree is + * being traversed in depth-first order, or -1 otherwise. */ +int part_num; + /* True if decryption of this part was attempted. */ notmuch_bool_t decrypt_attempted; /* True if decryption of this part's child succeeded. In this @@ -324,6 +331,11 @@ typedef struct mime_node { /* Internal: For successfully decrypted multipart parts, the * decrypted part to substitute for the second child. */ GMimeObject *decrypted_child; + +/* Internal: The next child for depth-first traversal and the part + * number to assign it (or -1 if unknown). */ +int next_child; +int next_part_num; } mime_node_t; /* Construct a new MIME node pointing to the root message part of @@ -356,7 +368,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message, * an error message on stderr). */ mime_node_t * -mime_node_child (const mime_node_t *parent, int child); +mime_node_child (mime_node_t *parent, int child); /* Return the nth child of node in a depth-first traversal. If n is * 0, returns node itself. Returns NULL if there is no such part. */ -- 1.7.7.3
[PATCH v5 0/2] Second step of 'show' rewrite
Arrg, sorry for the spam. I grabbed the wrong commit ID when sending v4.
[PATCH v4 3/3] show: Introduce mime_node formatter callback
This callback is the gateway to the new mime_node_t-based formatters. This maintains backwards compatibility so the formatters can be transitioned one at a time. Once all formatters are converted, the formatter structure can be reduced to only message_set_{start,sep,end} and part, most of show_message can be deleted, and all of show-message.c can be deleted. --- notmuch-client.h |6 ++ notmuch-reply.c |2 +- notmuch-show.c | 23 +++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/notmuch-client.h b/notmuch-client.h index abfe5d3..59606b4 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -62,8 +62,14 @@ #define STRINGIFY(s) STRINGIFY_(s) #define STRINGIFY_(s) #s +struct mime_node; +struct notmuch_show_params; + typedef struct notmuch_show_format { const char *message_set_start; +void (*part) (const void *ctx, + struct mime_node *node, int indent, + struct notmuch_show_params *params); const char *message_start; void (*message) (const void *ctx, notmuch_message_t *message, diff --git a/notmuch-reply.c b/notmuch-reply.c index bf67960..f55b1d2 100644 --- a/notmuch-reply.c +++ b/notmuch-reply.c @@ -31,7 +31,7 @@ static void reply_part_content (GMimeObject *part); static const notmuch_show_format_t format_reply = { -"", +"", NULL, "", NULL, "", NULL, reply_headers_message_part, ">\n", "", diff --git a/notmuch-show.c b/notmuch-show.c index 682aa71..8185b02 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -42,7 +42,7 @@ static void format_part_end_text (GMimeObject *part); static const notmuch_show_format_t format_text = { -"", +"", NULL, "\fmessage{ ", format_message_text, "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n", "\fbody{\n", @@ -89,7 +89,7 @@ static void format_part_end_json (GMimeObject *part); static const notmuch_show_format_t format_json = { -"[", +"[", NULL, "{", format_message_json, "\"headers\": {", format_headers_json, format_headers_message_part_json, "}", ", \"body\": [", @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx, unused (int indent)); static const notmuch_show_format_t format_mbox = { -"", +"", NULL, "", format_message_mbox, "", NULL, NULL, "", "", @@ -129,7 +129,7 @@ static void format_part_content_raw (GMimeObject *part); static const notmuch_show_format_t format_raw = { -"", +"", NULL, "", NULL, "", NULL, format_headers_message_part_text, "\n", "", @@ -850,6 +850,21 @@ show_message (void *ctx, int indent, notmuch_show_params_t *params) { +if (format->part) { + void *local = talloc_new (ctx); + mime_node_t *root, *part; + + if (mime_node_open (local, message, params->cryptoctx, params->decrypt, + &root) != NOTMUCH_STATUS_SUCCESS) + goto DONE; + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part); + if (part) + format->part (local, part, indent, params); + DONE: + talloc_free (local); + return; +} + if (params->part <= 0) { fputs (format->message_start, stdout); if (format->message) -- 1.7.7.3
[PATCH v4 2/3] show: Use consistent header ordering in the text format
Previously, top-level message headers were printed as Subject, From, To, Date, while embedded message headers were printed From, To, Subject, Date. This makes both cases use the former order and updates the tests accordingly. Strangely, the raw format also uses this function, so this also fixes the two raw format tests affected by this change. --- notmuch-show.c |2 +- test/multipart | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index 7b40568..682aa71 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -364,6 +364,7 @@ format_headers_message_part_text (GMimeMessage *message) InternetAddressList *recipients; const char *recipients_string; +printf ("Subject: %s\n", g_mime_message_get_subject (message)); printf ("From: %s\n", g_mime_message_get_sender (message)); recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO); recipients_string = internet_address_list_to_string (recipients, 0); @@ -375,7 +376,6 @@ format_headers_message_part_text (GMimeMessage *message) if (recipients_string) printf ("Cc: %s\n", recipients_string); -printf ("Subject: %s\n", g_mime_message_get_subject (message)); printf ("Date: %s\n", g_mime_message_get_date_as_string (message)); } diff --git a/test/multipart b/test/multipart index f83526b..2dd73f5 100755 --- a/test/multipart +++ b/test/multipart @@ -121,9 +121,9 @@ Date: Fri, 05 Jan 2001 15:43:57 + part{ ID: 2, Content-type: multipart/mixed part{ ID: 3, Content-type: message/rfc822 header{ +Subject: html message From: Carl Worth To: cworth at cworth.org -Subject: html message Date: Fri, 05 Jan 2001 15:42:57 + header} body{ @@ -162,9 +162,9 @@ catOU cat OUT # output should *not* include newline echo >>OUTPUT cat OUTPUT cat
[PATCH v4 1/3] mime node: Record depth-first part numbers
This makes the part numbers readily accessible to formatters. Hierarchical part numbering would be a more natural and efficient fit for MIME and may be the way to go in the future, but depth-first numbering maintains compatibility with what we currently do. --- mime-node.c | 37 ++--- notmuch-client.h | 14 +- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/mime-node.c b/mime-node.c index 27077f7..025c537 100644 --- a/mime-node.c +++ b/mime-node.c @@ -112,6 +112,10 @@ mime_node_open (const void *ctx, notmuch_message_t *message, root->nchildren = 1; root->ctx = mctx; +root->part_num = 0; +root->next_child = 0; +root->next_part_num = 1; + *root_out = root; return NOTMUCH_STATUS_SUCCESS; @@ -137,7 +141,7 @@ _signature_validity_free (GMimeSignatureValidity **proxy) #endif static mime_node_t * -_mime_node_create (const mime_node_t *parent, GMimeObject *part) +_mime_node_create (mime_node_t *parent, GMimeObject *part) { mime_node_t *node = talloc_zero (parent, mime_node_t); GError *err = NULL; @@ -150,6 +154,8 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) talloc_free (node); return NULL; } +node->parent = parent; +node->part_num = node->next_part_num = -1; /* Deal with the different types of parts */ if (GMIME_IS_PART (part)) { @@ -267,9 +273,10 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) } mime_node_t * -mime_node_child (const mime_node_t *parent, int child) +mime_node_child (mime_node_t *parent, int child) { GMimeObject *sub; +mime_node_t *node; if (!parent || child < 0 || child >= parent->nchildren) return NULL; @@ -287,7 +294,31 @@ mime_node_child (const mime_node_t *parent, int child) INTERNAL_ERROR ("Unexpected GMimeObject type: %s", g_type_name (G_OBJECT_TYPE (parent->part))); } -return _mime_node_create (parent, sub); +node = _mime_node_create (parent, sub); + +if (child == parent->next_child && parent->next_part_num != -1) { + /* We're traversing in depth-first order. Record the child's +* depth-first numbering. */ + node->part_num = parent->next_part_num; + node->next_part_num = node->part_num + 1; + + /* Drop the const qualifier because these are internal fields +* whose mutability doesn't affect the interface. */ + parent->next_child++; + parent->next_part_num = -1; + + if (node->nchildren == 0) { + /* We've reached a leaf, so find the parent that has more +* children and set it up to number its next child. */ + mime_node_t *it = node; + while (it && it->next_child == it->nchildren) + it = it->parent; + if (it) + it->next_part_num = node->part_num + 1; + } +} + +return node; } static mime_node_t * diff --git a/notmuch-client.h b/notmuch-client.h index 9c1d383..abfe5d3 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -297,6 +297,13 @@ typedef struct mime_node { /* The number of children of this part. */ int nchildren; +/* The parent of this node or NULL if this is the root node. */ +struct mime_node *parent; + +/* The depth-first part number of this child if the MIME tree is + * being traversed in depth-first order, or -1 otherwise. */ +int part_num; + /* True if decryption of this part was attempted. */ notmuch_bool_t decrypt_attempted; /* True if decryption of this part's child succeeded. In this @@ -324,6 +331,11 @@ typedef struct mime_node { /* Internal: For successfully decrypted multipart parts, the * decrypted part to substitute for the second child. */ GMimeObject *decrypted_child; + +/* Internal: The next child for depth-first traversal and the part + * number to assign it (or -1 if unknown). */ +int next_child; +int next_part_num; } mime_node_t; /* Construct a new MIME node pointing to the root message part of @@ -356,7 +368,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message, * an error message on stderr). */ mime_node_t * -mime_node_child (const mime_node_t *parent, int child); +mime_node_child (mime_node_t *parent, int child); /* Return the nth child of node in a depth-first traversal. If n is * 0, returns node itself. Returns NULL if there is no such part. */ -- 1.7.7.3
[PATCH v4 0/3] Second step of 'show' rewrite
This version addresses Dmitry's review.
[PATCH v2 3/3] show: Introduce mime_node formatter callback
Thanks for the review. New version coming shortly (v4, since the list ate v3 while everyone was still reading v2). Quoth Dmitry Kurochkin on Jan 24 at 2:37 am: > On Sun, 22 Jan 2012 21:31:13 -0500, Austin Clements > wrote: > > This callback is the gateway to the new mime_node_t-based formatters. > > This maintains backwards compatibility so the formatters can be > > transitioned one at a time. Once all formatters are converted, the > > formatter structure can be reduced to only message_set_{start,sep,end} > > and part, most of show_message can be deleted, and all of > > show-message.c can be deleted. > > Few comments below. > > > --- > > notmuch-client.h |6 ++ > > notmuch-reply.c |2 +- > > notmuch-show.c | 23 +++ > > 3 files changed, 26 insertions(+), 5 deletions(-) > > > > diff --git a/notmuch-client.h b/notmuch-client.h > > index abfe5d3..59606b4 100644 > > --- a/notmuch-client.h > > +++ b/notmuch-client.h > > @@ -62,8 +62,14 @@ > > #define STRINGIFY(s) STRINGIFY_(s) > > #define STRINGIFY_(s) #s > > > > +struct mime_node; > > +struct notmuch_show_params; > > + > > typedef struct notmuch_show_format { > > const char *message_set_start; > > +void (*part) (const void *ctx, > > + struct mime_node *node, int indent, > > + struct notmuch_show_params *params); > > Can we make the params parameter const? Apparently we can. > > const char *message_start; > > void (*message) (const void *ctx, > > notmuch_message_t *message, > > diff --git a/notmuch-reply.c b/notmuch-reply.c > > index bf67960..f55b1d2 100644 > > --- a/notmuch-reply.c > > +++ b/notmuch-reply.c > > @@ -31,7 +31,7 @@ static void > > reply_part_content (GMimeObject *part); > > > > static const notmuch_show_format_t format_reply = { > > -"", > > +"", NULL, > > "", NULL, > > "", NULL, reply_headers_message_part, ">\n", > > "", > > diff --git a/notmuch-show.c b/notmuch-show.c > > index 682aa71..8185b02 100644 > > --- a/notmuch-show.c > > +++ b/notmuch-show.c > > @@ -42,7 +42,7 @@ static void > > format_part_end_text (GMimeObject *part); > > > > static const notmuch_show_format_t format_text = { > > -"", > > +"", NULL, > > "\fmessage{ ", format_message_text, > > "\fheader{\n", format_headers_text, > > format_headers_message_part_text, "\fheader}\n", > > "\fbody{\n", > > @@ -89,7 +89,7 @@ static void > > format_part_end_json (GMimeObject *part); > > > > static const notmuch_show_format_t format_json = { > > -"[", > > +"[", NULL, > > "{", format_message_json, > > "\"headers\": {", format_headers_json, > > format_headers_message_part_json, "}", > > ", \"body\": [", > > @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx, > > unused (int indent)); > > > > static const notmuch_show_format_t format_mbox = { > > -"", > > +"", NULL, > > "", format_message_mbox, > > "", NULL, NULL, "", > > "", > > @@ -129,7 +129,7 @@ static void > > format_part_content_raw (GMimeObject *part); > > > > static const notmuch_show_format_t format_raw = { > > -"", > > +"", NULL, > > "", NULL, > > "", NULL, format_headers_message_part_text, "\n", > > "", > > @@ -850,6 +850,21 @@ show_message (void *ctx, > > int indent, > > notmuch_show_params_t *params) > > { > > +if (format->part) { > > + void *local = talloc_new (ctx); > > + mime_node_t *root, *part; > > + > > + if (mime_node_open (local, message, params->cryptoctx, params->decrypt, > > + &root) != NOTMUCH_STATUS_SUCCESS) > > + goto DONE; > > + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part); > > This should be done as a separate patch, but it looks like zero and > negative params->part is handled in the same way so we can change it to > always be non-negative. That's true here, but there are other places where the difference does matter. (I would certainly *prefer* this not to be asymmetric, but that's a bigger issue involving show's inconsistent interface.) > > + if (part) > > + format->part (local, part, indent, params); > > + DONE: > > + talloc_free (local); > > + return; > > Please move part assignment inside the if and remove the goto: > > if (mime_node_open() == success && (part = seek())) > format->part(); Done. > Regards, > Dmitry > > > +} > > + > > if (params->part <= 0) { > > fputs (format->message_start, stdout); > > if (format->message) >
[PATCH v2 1/3] mime node: Record depth-first part numbers
Quoth Dmitry Kurochkin on Jan 24 at 2:19 am: > On Sun, 22 Jan 2012 21:31:11 -0500, Austin Clements > wrote: > > This makes the part numbers readily accessible to formatters. > > Hierarchical part numbering would be a more natural and efficient fit > > for MIME and may be the way to go in the future, but depth-first > > numbering maintains compatibility with what we currently do. > > The patch looks good except for few minor comments below. I do not > think that we need another review for the next patch version. > > > --- > > mime-node.c | 37 ++--- > > notmuch-client.h | 14 +- > > 2 files changed, 47 insertions(+), 4 deletions(-) > > > > diff --git a/mime-node.c b/mime-node.c > > index 27077f7..025c537 100644 > > --- a/mime-node.c > > +++ b/mime-node.c > > @@ -112,6 +112,10 @@ mime_node_open (const void *ctx, notmuch_message_t > > *message, > > root->nchildren = 1; > > root->ctx = mctx; > > > > +root->part_num = 0; > > +root->next_child = 0; > > +root->next_part_num = 1; > > + > > For consistency, we should either initialize root->parent to NULL > explicitly or remove part_num and next_child initialization. > > > *root_out = root; > > return NOTMUCH_STATUS_SUCCESS; > > > > @@ -137,7 +141,7 @@ _signature_validity_free (GMimeSignatureValidity > > **proxy) > > #endif > > > > static mime_node_t * > > -_mime_node_create (const mime_node_t *parent, GMimeObject *part) > > +_mime_node_create (mime_node_t *parent, GMimeObject *part) > > { > > mime_node_t *node = talloc_zero (parent, mime_node_t); > > GError *err = NULL; > > @@ -150,6 +154,8 @@ _mime_node_create (const mime_node_t *parent, > > GMimeObject *part) > > talloc_free (node); > > return NULL; > > } > > +node->parent = parent; > > +node->part_num = node->next_part_num = -1; > > Same here: if we initialize next_child to zero explicitly in > mime_node_open(), we should do it here as well. Both done. > > > > /* Deal with the different types of parts */ > > if (GMIME_IS_PART (part)) { > > @@ -267,9 +273,10 @@ _mime_node_create (const mime_node_t *parent, > > GMimeObject *part) > > } > > > > mime_node_t * > > -mime_node_child (const mime_node_t *parent, int child) > > +mime_node_child (mime_node_t *parent, int child) > > { > > GMimeObject *sub; > > +mime_node_t *node; > > > > if (!parent || child < 0 || child >= parent->nchildren) > > return NULL; > > @@ -287,7 +294,31 @@ mime_node_child (const mime_node_t *parent, int child) > > INTERNAL_ERROR ("Unexpected GMimeObject type: %s", > > g_type_name (G_OBJECT_TYPE (parent->part))); > > } > > -return _mime_node_create (parent, sub); > > +node = _mime_node_create (parent, sub); > > + > > +if (child == parent->next_child && parent->next_part_num != -1) { > > + /* We're traversing in depth-first order. Record the child's > > +* depth-first numbering. */ > > + node->part_num = parent->next_part_num; > > + node->next_part_num = node->part_num + 1; > > + > > + /* Drop the const qualifier because these are internal fields > > +* whose mutability doesn't affect the interface. */ > > The comment is no longer relevant, please remove. > > > + parent->next_child++; > > + parent->next_part_num = -1; > > + > > + if (node->nchildren == 0) { > > + /* We've reached a leaf, so find the parent that has more > > +* children and set it up to number its next child. */ > > + mime_node_t *it = node; > > + while (it && it->next_child == it->nchildren) > > It seems that it should be initialized to node->parent, because > node->next_child is always 0. Either works. I started at node because it seemed like a more fundamental base case, but perhaps that just makes this code even more obtuse. > Just curious, does "it" stands for "iterator"? I would prefer just "i" > or "iter" :) "it" is a C++ habit. I changed it to "iter". > > + it = it->parent; > > + if (it) > > + it->next_part_num = node->part_num + 1; > > + } > > +} > > Austin, I trust you that this code works :) Though I hope we will get > to hierarchical part numbering one day and it would simplify this code. It would simplify it down to one line, in fact. Code simplification aside, I think hierarchical numbering is the right thing to do. But that's for another day. > Regards, > Dmitry > > > + > > +return node; > > } > > > > static mime_node_t * > > diff --git a/notmuch-client.h b/notmuch-client.h > > index 9c1d383..abfe5d3 100644 > > --- a/notmuch-client.h > > +++ b/notmuch-client.h > > @@ -297,6 +297,13 @@ typedef struct mime_node { > > /* The number of children of this part. */ > > int nchildren; > > > > +/* The parent of this node or NULL if this is the root node. */ > > +struct mime_node *parent; > > + > > +/* The depth-first part number of
[PATCH 4/4 v42] test: Add address cleaning tests.
--- Anticipate the failure of test 3. test/emacs-address-cleaning.el | 29 + test/emacs-address-cleaning.sh | 19 +++ test/notmuch-test |1 + 3 files changed, 49 insertions(+), 0 deletions(-) create mode 100644 test/emacs-address-cleaning.el create mode 100755 test/emacs-address-cleaning.sh diff --git a/test/emacs-address-cleaning.el b/test/emacs-address-cleaning.el new file mode 100644 index 000..59e8d92 --- /dev/null +++ b/test/emacs-address-cleaning.el @@ -0,0 +1,29 @@ +(defun notmuch-test-address-cleaning-1 () + (notmuch-test-compare (notmuch-show-clean-address "dme at dme.org") + "dme at dme.org")) + +(defun notmuch-test-address-cleaning-2 () + (let* ((input '("foo at bar.com" + "" + "Foo Bar " + "foo at bar.com " + "\"Foo Bar\" ")) +(expected '("foo at bar.com" +"foo at bar.com" +"Foo Bar " +"foo at bar.com" +"Foo Bar ")) +(output (mapcar #'notmuch-show-clean-address input))) +(notmuch-test-compare output expected))) + +(defun notmuch-test-address-cleaning-3 () + (let* ((input '("?? " + "foo (at home) " + "foo [at home] " + "Foo Bar")) +(expected '("?? " +"foo (at home) " +"foo [at home] " +"Foo Bar")) +(output (mapcar #'notmuch-show-clean-address input))) +(notmuch-test-compare output expected))) diff --git a/test/emacs-address-cleaning.sh b/test/emacs-address-cleaning.sh new file mode 100755 index 000..0d85bdc --- /dev/null +++ b/test/emacs-address-cleaning.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash + +test_description="emacs address cleaning" +. test-lib.sh + +test_begin_subtest "notmuch-test-address-clean part 1" +test_emacs_expect_t \ +'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-1)' + +test_begin_subtest "notmuch-test-address-clean part 2" +test_emacs_expect_t \ +'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-2)' + +test_begin_subtest "notmuch-test-address-clean part 3" +test_subtest_known_broken +test_emacs_expect_t \ +'(load "emacs-address-cleaning.el") (notmuch-test-address-cleaning-3)' + +test_done diff --git a/test/notmuch-test b/test/notmuch-test index d034f99..3f1740c 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -53,6 +53,7 @@ TESTS=" hooks argument-parsing emacs-test-functions.sh + emacs-address-cleaning.sh " TESTS=${NOTMUCH_TESTS:=$TESTS} -- 1.7.8.3
[PATCH 3/4 v42] test: Add more helpers for emacs tests.
--- Split out from the tests and re-factored. test/test-lib.el | 22 ++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/test/test-lib.el b/test/test-lib.el index 96752f0..c4a5db4 100644 --- a/test/test-lib.el +++ b/test/test-lib.el @@ -20,6 +20,8 @@ ;; ;; Authors: Dmitry Kurochkin +(require 'cl) ;; This code is generally used uncompiled. + ;; `read-file-name' by default uses `completing-read' function to read ;; user input. It does not respect `standard-input' variable which we ;; use in tests to provide user input. So replace it with a plain @@ -92,3 +94,23 @@ nothing." result (prin1-to-string result))) (test-output + +(defun notmuch-test-report-unexpected (output expected) + "Report that the OUTPUT does not match the EXPECTED result." + (concat "Expect:\t" (prin1-to-string expected) "\n" + "Output:\t" (prin1-to-string output) "\n")) + +(defun notmuch-test-compare (output expected) + "Compare OUTPUT with EXPECTED. Report any discrepencies." + (if (equal output expected) + t +(cond + ((and (listp output) + (listp expected)) + (apply #'concat (loop for o in output + for e in expected + if (not (equal o e)) + collect (notmuch-test-report-unexpected o e + + (t + (notmuch-test-report-unexpected output expected) -- 1.7.8.3
[PATCH 2/4 v42] test: Add `test_emacs_expect_t'.
Add a new test function to allow simpler testing of emacs functionality. `test_emacs_expect_t' takes one argument - a lisp expression to evaluate. The test passes if the expression returns `t', otherwise it fails and the output is reported to the tester. --- As per Dmitry: - don't call `test_skip' twice, - allow for a prereq. test/README |8 test/emacs-test-functions.sh |9 + test/notmuch-test|1 + test/test-lib.el |9 + test/test-lib.sh | 29 + 5 files changed, 56 insertions(+), 0 deletions(-) create mode 100755 test/emacs-test-functions.sh diff --git a/test/README b/test/README index 44ff653..43656a3 100644 --- a/test/README +++ b/test/README @@ -202,6 +202,14 @@ library for your script to use. tests that may run in the same Emacs instance. Use `let' instead so the scope of the changed variables is limited to a single test. + test_emacs_expect_t + + This function executes the provided emacs lisp script within + emacs in a manner similar to 'test_emacs'. The expressions should + return the value `t' to indicate that the test has passed. If the + test does not return `t' then it is considered failed and all data + returned by the test is reported to the tester. + test_done Your test script must have test_done at the end. Its purpose diff --git a/test/emacs-test-functions.sh b/test/emacs-test-functions.sh new file mode 100755 index 000..0e1f9fc --- /dev/null +++ b/test/emacs-test-functions.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +test_description="emacs test function sanity" +. test-lib.sh + +test_begin_subtest "emacs test function sanity" +test_emacs_expect_t 't' + +test_done diff --git a/test/notmuch-test b/test/notmuch-test index 6a99ae3..d034f99 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -52,6 +52,7 @@ TESTS=" python hooks argument-parsing + emacs-test-functions.sh " TESTS=${NOTMUCH_TESTS:=$TESTS} diff --git a/test/test-lib.el b/test/test-lib.el index 59c5868..96752f0 100644 --- a/test/test-lib.el +++ b/test/test-lib.el @@ -83,3 +83,12 @@ nothing." (add-hook-counter 'notmuch-hello-mode-hook) (add-hook-counter 'notmuch-hello-refresh-hook) + +(defmacro notmuch-test-run (&rest body) + "Evaluate a BODY of test expressions and output the result." + `(with-temp-buffer + (let ((result (progn , at body))) + (insert (if (stringp result) + result +(prin1-to-string result))) + (test-output diff --git a/test/test-lib.sh b/test/test-lib.sh index 82c686c..8158328 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -503,6 +503,35 @@ test_expect_equal_file () fi } +test_emacs_expect_t () { + test "$#" = 2 && { prereq=$1; shift; } || prereq= + test "$#" = 1 || + error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t" + + # Run the test. + if ! test_skip "$test_subtest_name" + then + test_emacs "(notmuch-test-run $1)" >/dev/null + + # Restore state after the test. + exec 1>&6 2>&7 # Restore stdout and stderr + inside_subtest= + + # Report success/failure. + result=$(cat OUTPUT) + if [ "$result" = t ] + then + test_ok_ "$test_subtest_name" + else + test_failure_ "$test_subtest_name" "${result}" + fi + else + # Restore state after the (non) test. + exec 1>&6 2>&7 # Restore stdout and stderr + inside_subtest= + fi +} + NOTMUCH_NEW () { notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file' -- 1.7.8.3
[PATCH 1/4 v42] test: Don't return the result of checking for running emacs to the tester.
When checking for a running emacs, test_emacs evaluates the empty list '()'. This returns 'nil' when emacs is running, which is then prepended to the actual test result. Given that it is not part of the actual test output the test harness can incorrectly report test failure (or success). --- Commentary updated. test/test-lib.sh |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/test/test-lib.sh b/test/test-lib.sh index 0da60fb..82c686c 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -926,7 +926,7 @@ test_emacs () { --eval '(orphan-watchdog $$)'" || return EMACS_SERVER="$server_name" # wait until the emacs server is up - until test_emacs '()' 2>/dev/null; do + until test_emacs '()' >/dev/null 2>/dev/null; do sleep 1 done fi -- 1.7.8.3
[PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
On Mon, 23 Jan 2012 09:39:30 -0800, Jameson Graef Rollins wrote: > On Mon, 23 Jan 2012 11:02:04 +, David Edmondson wrote: > > On Mon, 23 Jan 2012 00:34:23 -0800, Jameson Graef Rollins > finestructure.net> wrote: > > > -(defun notmuch-show-next-open-message () > > > +(defun notmuch-show-next-open-message (&optional pop-at-end) > > >"Show the next message." > > > - (interactive) > > > - (let (r) > > > + (interactive "P") > > > + (let ((parent-buffer notmuch-show-parent-buffer) > > > + (r)) > > > > Extra brackets around 'r' are unnecessary. Otherwise, +1. > > This seems minor enough that I'm not going to resend, but noted for next > time. Thanks. On that basis, I hope no-one will complain if I fix them as a 'drive by' during another change... -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/e6d41843/attachment-0001.pgp>
[RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray wrote: > > +;; Nonsense required to have the new gnus `shr' HTML display code > > +;; work. > > +(defvar gnus-summary-buffer) > > +(defvar gnus-inhibit-images) > > I believe that gnus-inhibit-images is defined in gnus-art.el. Perhaps > that file should just be require'd rather than having this workaround. With notmuch-0.11 (require 'gnus-art') "fixes" the void gnus-inhibit-images error. > > +(if (not (fboundp 'gnus-blocked-images)) > > +(defun gnus-blocked-images () nil)) > > + > > (defun notmuch-show-mm-display-part-inline (msg part nth content-type) > >"Use the mm-decode/mm-view functions to display a part in the > > current buffer, if possible." > > @@ -331,7 +338,12 @@ current buffer, if possible." > > (let ((content (notmuch-show-get-bodypart-content msg part nth))) > > (insert content) > > (set-buffer display-buffer) > > - (mm-display-part handle) > > + > > + ;; Nonsense required to have the new gnus `shr' HTML > > + ;; display code work. > > + (let ((gnus-inhibit-images nil)) > > + (makunbound 'gnus-summary-buffer) ; Blech. > > This is working around a bug in gnus. I think the better solution would > be for gnus to fix the bug. The following patch against gnus works for > me. (I have tried submitting it to the gnus bug list, but have not been > able to check if it got through.) emacs git master has a fix for that, so you seem to have got through. -- Florian Friesdorf GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083 Jabber/XMPP: flo at chaoflow.net IRC: chaoflow on freenode,ircnet,blafasel,OFTC
Infinite loop in emacs interface
On Mon, 23 Jan 2012 15:26:20 +0100, Florian Friesdorf wrote: > On Tue, 17 Jan 2012 23:44:40 +0100, Rodney Lorrimar > wrote: > > > > Debugger entered--Lisp error: (void-variable gnus-inhibit-images) > > mm-shr((# ("text/html") nil nil nil nil nil nil)) > > mm-inline-text-html((# ("text/html") nil nil nil nil nil > > nil)) > > mm-display-inline((# ("text/html") nil nil nil nil nil > > nil)) > > mm-display-part((# ("text/html") nil nil nil nil nil nil)) > > notmuch-show-mm-display-part-inline(...snipped...) > > notmuch-search-show-thread(nil) > > call-interactively(notmuch-search-show-thread nil nil) > > With emacs from git master as of today > (1f48de2bbf78c87c1dee6425181b31be60d39c7c) and notmuch-0.11 you can > workaround it: > > (require 'gnus-art) > > With emacs pretest 24.0.92 there is a bug in gnus. > > Fixes/workarounds are discussed in: > "[PATCH] emacs: Make the part content available to `mm-inline* checks" sorry, was discussed in: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run. -- Florian Friesdorf GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083 Jabber/XMPP: flo at chaoflow.net IRC: chaoflow on freenode,ircnet,blafasel,OFTC
[PATCH v3 2/2] show: Introduce mime_node formatter callback
This callback is the gateway to the new mime_node_t-based formatters. This maintains backwards compatibility so the formatters can be transitioned one at a time. Once all formatters are converted, the formatter structure can be reduced to only message_set_{start,sep,end} and part, most of show_message can be deleted, and all of show-message.c can be deleted. --- notmuch-client.h |6 ++ notmuch-reply.c |2 +- notmuch-show.c | 23 +++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/notmuch-client.h b/notmuch-client.h index abfe5d3..59606b4 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -62,8 +62,14 @@ #define STRINGIFY(s) STRINGIFY_(s) #define STRINGIFY_(s) #s +struct mime_node; +struct notmuch_show_params; + typedef struct notmuch_show_format { const char *message_set_start; +void (*part) (const void *ctx, + struct mime_node *node, int indent, + struct notmuch_show_params *params); const char *message_start; void (*message) (const void *ctx, notmuch_message_t *message, diff --git a/notmuch-reply.c b/notmuch-reply.c index bf67960..f55b1d2 100644 --- a/notmuch-reply.c +++ b/notmuch-reply.c @@ -31,7 +31,7 @@ static void reply_part_content (GMimeObject *part); static const notmuch_show_format_t format_reply = { -"", +"", NULL, "", NULL, "", NULL, reply_headers_message_part, ">\n", "", diff --git a/notmuch-show.c b/notmuch-show.c index 682aa71..8185b02 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -42,7 +42,7 @@ static void format_part_end_text (GMimeObject *part); static const notmuch_show_format_t format_text = { -"", +"", NULL, "\fmessage{ ", format_message_text, "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n", "\fbody{\n", @@ -89,7 +89,7 @@ static void format_part_end_json (GMimeObject *part); static const notmuch_show_format_t format_json = { -"[", +"[", NULL, "{", format_message_json, "\"headers\": {", format_headers_json, format_headers_message_part_json, "}", ", \"body\": [", @@ -110,7 +110,7 @@ format_message_mbox (const void *ctx, unused (int indent)); static const notmuch_show_format_t format_mbox = { -"", +"", NULL, "", format_message_mbox, "", NULL, NULL, "", "", @@ -129,7 +129,7 @@ static void format_part_content_raw (GMimeObject *part); static const notmuch_show_format_t format_raw = { -"", +"", NULL, "", NULL, "", NULL, format_headers_message_part_text, "\n", "", @@ -850,6 +850,21 @@ show_message (void *ctx, int indent, notmuch_show_params_t *params) { +if (format->part) { + void *local = talloc_new (ctx); + mime_node_t *root, *part; + + if (mime_node_open (local, message, params->cryptoctx, params->decrypt, + &root) != NOTMUCH_STATUS_SUCCESS) + goto DONE; + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part); + if (part) + format->part (local, part, indent, params); + DONE: + talloc_free (local); + return; +} + if (params->part <= 0) { fputs (format->message_start, stdout); if (format->message) -- 1.7.7.3
[PATCH v3 1/2] mime node: Record depth-first part numbers
This makes the part numbers readily accessible to formatters. Hierarchical part numbering would be a more natural and efficient fit for MIME and may be the way to go in the future, but depth-first numbering maintains compatibility with what we currently do. --- mime-node.c | 36 +--- notmuch-client.h | 14 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/mime-node.c b/mime-node.c index 27077f7..c073c7e 100644 --- a/mime-node.c +++ b/mime-node.c @@ -112,6 +112,10 @@ mime_node_open (const void *ctx, notmuch_message_t *message, root->nchildren = 1; root->ctx = mctx; +root->part_num = 0; +root->next_child = 0; +root->next_part_num = 1; + *root_out = root; return NOTMUCH_STATUS_SUCCESS; @@ -137,7 +141,7 @@ _signature_validity_free (GMimeSignatureValidity **proxy) #endif static mime_node_t * -_mime_node_create (const mime_node_t *parent, GMimeObject *part) +_mime_node_create (mime_node_t *parent, GMimeObject *part) { mime_node_t *node = talloc_zero (parent, mime_node_t); GError *err = NULL; @@ -150,6 +154,8 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) talloc_free (node); return NULL; } +node->parent = parent; +node->part_num = node->next_part_num = -1; /* Deal with the different types of parts */ if (GMIME_IS_PART (part)) { @@ -267,9 +273,10 @@ _mime_node_create (const mime_node_t *parent, GMimeObject *part) } mime_node_t * -mime_node_child (const mime_node_t *parent, int child) +mime_node_child (mime_node_t *parent, int child) { GMimeObject *sub; +mime_node_t *node; if (!parent || child < 0 || child >= parent->nchildren) return NULL; @@ -287,7 +294,30 @@ mime_node_child (const mime_node_t *parent, int child) INTERNAL_ERROR ("Unexpected GMimeObject type: %s", g_type_name (G_OBJECT_TYPE (parent->part))); } -return _mime_node_create (parent, sub); +node = _mime_node_create (parent, sub); + +if (child == parent->next_child && parent->next_part_num != -1) { + /* We're traversing in depth-first order. Record the child's +* depth-first numbering. */ + node->part_num = parent->next_part_num; + node->next_part_num = node->part_num + 1; + + /* Prepare the parent for its next depth-first child. */ + parent->next_child++; + parent->next_part_num = -1; + + if (node->nchildren == 0) { + /* We've reached a leaf, so find the parent that has more +* children and set it up to number its next child. */ + mime_node_t *it = node; + while (it && it->next_child == it->nchildren) + it = it->parent; + if (it) + it->next_part_num = node->part_num + 1; + } +} + +return node; } static mime_node_t * diff --git a/notmuch-client.h b/notmuch-client.h index 9c1d383..abfe5d3 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -297,6 +297,13 @@ typedef struct mime_node { /* The number of children of this part. */ int nchildren; +/* The parent of this node or NULL if this is the root node. */ +struct mime_node *parent; + +/* The depth-first part number of this child if the MIME tree is + * being traversed in depth-first order, or -1 otherwise. */ +int part_num; + /* True if decryption of this part was attempted. */ notmuch_bool_t decrypt_attempted; /* True if decryption of this part's child succeeded. In this @@ -324,6 +331,11 @@ typedef struct mime_node { /* Internal: For successfully decrypted multipart parts, the * decrypted part to substitute for the second child. */ GMimeObject *decrypted_child; + +/* Internal: The next child for depth-first traversal and the part + * number to assign it (or -1 if unknown). */ +int next_child; +int next_part_num; } mime_node_t; /* Construct a new MIME node pointing to the root message part of @@ -356,7 +368,7 @@ mime_node_open (const void *ctx, notmuch_message_t *message, * an error message on stderr). */ mime_node_t * -mime_node_child (const mime_node_t *parent, int child); +mime_node_child (mime_node_t *parent, int child); /* Return the nth child of node in a depth-first traversal. If n is * 0, returns node itself. Returns NULL if there is no such part. */ -- 1.7.7.3
[PATCH v3 0/2] Second step of 'show' rewrite
Fix a silly comment mistake pointed out by Jani
Infinite loop in emacs interface
On Tue, 17 Jan 2012 23:44:40 +0100, Rodney Lorrimar wrote: > > Debugger entered--Lisp error: (void-variable gnus-inhibit-images) > mm-shr((# ("text/html") nil nil nil nil nil nil)) > mm-inline-text-html((# ("text/html") nil nil nil nil nil > nil)) > mm-display-inline((# ("text/html") nil nil nil nil nil nil)) > mm-display-part((# ("text/html") nil nil nil nil nil nil)) > notmuch-show-mm-display-part-inline(...snipped...) > notmuch-search-show-thread(nil) > call-interactively(notmuch-search-show-thread nil nil) With emacs from git master as of today (1f48de2bbf78c87c1dee6425181b31be60d39c7c) and notmuch-0.11 you can workaround it: (require 'gnus-art) With emacs pretest 24.0.92 there is a bug in gnus. Fixes/workarounds are discussed in: "[PATCH] emacs: Make the part content available to `mm-inline* checks" -- Florian Friesdorf GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083 Jabber/XMPP: flo at chaoflow.net IRC: chaoflow on freenode,ircnet,blafasel,OFTC
[PATCH] Add a NEWS section for 0.11.1 and document the python error handling bugfix
--- NEWS | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/NEWS b/NEWS index bf21e64..3d2c2a8 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,17 @@ +Notmuch 0.11.1 (2012-mm-dd) +=== + +Bug-fix release. + + +Fix error handling in python bindings. + + The python bindings in 0.11 failed to detect NULL pointers being + returned from libnotmuch functions and thus failed to raise + exceptions to indicate the error condition. Any subsequent calls + into libnotmuch caused segmentation faults. + + Notmuch 0.11 (2012-01-13) = -- 1.7.8.3
[PATCH] python: fix error handling
On Sun, 22 Jan 2012 14:09:35 +0100, Justus Winter <4winter at informatik.uni-hamburg.de> wrote: > Before 3434d1940 the return values of libnotmuch functions were > declared as c_void_p and the code checking for errors compared the > returned value to None, which is the ctypes equivalent of a NULL > pointer. > > But said commit wrapped all the data types in python classes and the > semantic changed in a subtle way. If a function returns NULL, the > wrapped python value is falsish, but no longer equal to None. > > Backported from master to 0.11. > --- LGTM. Tomi
[PATCH 2/3] test: Add `test_emacs_expect_t'.
On Thu, 19 Jan 2012 12:54:02 +, David Edmondson wrote: > Add a new test function to allow simpler testing of emacs > functionality. > > `test_emacs_expect_t' takes one argument - a list expression to > evaluate. The test passes if the expression returns `t', otherwise it > fails and the output is reported to the tester. I've dealt with all of the comments for this patchset that I received with one exception (declaring test 3 as failing in patch 3). Any other comments before I submit another version? -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/b7bcf596/attachment.pgp>
[PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
v2 of these patches need to be merged with Mark's part saving stuff. I've done this, but I'll hold off posting the merged version until this one is reviewed, unless anyone objects. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/2115e78c/attachment.pgp>
[PATCH 6/6] emacs: modify the default show-mode key bindings for archiving
+1. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/eb85c7b1/attachment.pgp>
[PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
On Mon, 23 Jan 2012 00:34:23 -0800, Jameson Graef Rollins wrote: > -(defun notmuch-show-next-open-message () > +(defun notmuch-show-next-open-message (&optional pop-at-end) >"Show the next message." > - (interactive) > - (let (r) > + (interactive "P") > + (let ((parent-buffer notmuch-show-parent-buffer) > + (r)) Extra brackets around 'r' are unnecessary. Otherwise, +1. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/00a2e2c9/attachment-0001.pgp>
[PATCH 3/6] emacs: add message archiving functions
+1. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/ec84c3b6/attachment.pgp>
[PATCH 2/6] emacs: break out thread navigation from notmuch-show-archive-thread
+1. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/c68d3110/attachment.pgp>
[PATCH 1/6] emacs: break up notmuch-show-archive-thread-internal into two more generally useful functions
On Mon, 23 Jan 2012 00:34:20 -0800, Jameson Graef Rollins wrote: > Brake up notmuch-show-archive-thread-internal into two new functions: "Break", otherwise +1. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/2a65fda1/attachment.pgp>
[PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
On Mon, 23 Jan 2012 17:55:14 +, David Edmondson wrote: > On that basis, I hope no-one will complain if I fix them as a 'drive by' > during another change... Hrm. I don't think that follows. Unless the patch is already touching that code, I would really prefer we continue to enforce that unrelated changes go into separate patches. There are plenty of other places in the code where we do use parens around uninitialized variables. If you really want to remove them I would prefer to see a patch to fixes them all at once. Doesn't seem worth it to me, but everyone seems to be on an uncrustify kick recently, so... 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/20120123/245d8309/attachment-0001.pgp>
[PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/*.
Previously this logic applied only to text/plain. Allow it for other text/* parts as well. --- This was not included in version 1 of the patch set. emacs/notmuch-show.el |9 - 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 39f35ed..55e4e34 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -300,7 +300,9 @@ CONTENT-TYPE parts." (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment &rest button-parameters) - (unless (notmuch-show-hidden-part-header content-type) + (unless (or (notmuch-show-hidden-part-header content-type) + (and (= nth 1) + (string-match "text/*" content-type))) (apply #'insert-button (concat "[ " (if name (concat name ": ") "") @@ -561,10 +563,7 @@ current buffer, if possible." (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type) (let ((start (point))) -;; If this text/plain part is not the first part in the message, -;; insert a header to make this clear. -(if (> nth 1) - (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))) +(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)) (insert (notmuch-show-get-bodypart-content msg part nth)) (save-excursion (save-restriction -- 1.7.8.3
[PATCH 2/3 v2] emacs: Optionally hide some part headers.
Add a regexp, `notmuch-show-part-headers-hidden' and if the content-type of a part matches, don't show the part header. --- emacs/notmuch-show.el | 41 +++-- 1 files changed, 27 insertions(+), 14 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 9144484..39f35ed 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -110,6 +110,12 @@ indentation." :group 'notmuch :type 'boolean) +(defcustom notmuch-show-part-headers-hidden nil + "Headers for parts whose content-type matches this regexp will +not be shown." + :group 'notmuch + :type 'regexp) + (defmacro with-current-notmuch-show-message (&rest body) "Evaluate body with current buffer set to the text of current message" `(save-excursion @@ -285,23 +291,30 @@ message at DEPTH in the current thread." 'follow-link t 'face 'message-mml) +(defun notmuch-show-hidden-part-header (content-type) + "Return non-nil if a part header should be hidden for +CONTENT-TYPE parts." + (and notmuch-show-part-headers-hidden + (string-match notmuch-show-part-headers-hidden content-type))) + (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment &rest button-parameters) - (apply #'insert-button -(concat "[ " -(if name (concat name ": ") "") -declared-type -(if (not (string-equal declared-type content-type)) -(concat " (as " content-type ")") - "") -(or comment "") -" ]") -:type 'notmuch-show-part-button-type -:notmuch-part nth -:notmuch-filename name -button-parameters) - (insert "\n")) + (unless (notmuch-show-hidden-part-header content-type) +(apply #'insert-button + (concat "[ " + (if name (concat name ": ") "") + declared-type + (if (not (string-equal declared-type content-type)) + (concat " (as " content-type ")") +"") + (or comment "") + " ]") + :type 'notmuch-show-part-button-type + :notmuch-part nth + :notmuch-filename name + button-parameters) +(insert "\n"))) ;; Functions handling particular MIME parts. -- 1.7.8.3
[PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
Instead, allow the caller to specify some parameters for the button. Rework `notmuch-show-insert-part-multipart/signed' and `notmuch-show-insert-part-multipart/encrypted' accordingly. --- Removed the merge of multipart/signed and multipart/encrypted. emacs/notmuch-show.el | 84 +--- 1 files changed, 44 insertions(+), 40 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 03c1f6b..9144484 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -285,24 +285,23 @@ message at DEPTH in the current thread." 'follow-link t 'face 'message-mml) -(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment) - (let ((button)) -(setq button - (insert-button - (concat "[ " - (if name (concat name ": ") "") - declared-type - (if (not (string-equal declared-type content-type)) - (concat " (as " content-type ")") -"") - (or comment "") - " ]") - :type 'notmuch-show-part-button-type - :notmuch-part nth - :notmuch-filename name)) -(insert "\n") -;; return button -button)) +(defun notmuch-show-insert-part-header (nth content-type declared-type + &optional name comment + &rest button-parameters) + (apply #'insert-button +(concat "[ " +(if name (concat name ": ") "") +declared-type +(if (not (string-equal declared-type content-type)) +(concat " (as " content-type ")") + "") +(or comment "") +" ]") +:type 'notmuch-show-part-button-type +:notmuch-part nth +:notmuch-filename name +button-parameters) + (insert "\n")) ;; Functions handling particular MIME parts. @@ -460,15 +459,18 @@ current buffer, if possible." t) (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add signature status button if sigstatus provided -(if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) - (sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from)) - ;; if we're not adding sigstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))) + ;; Add signature status button if sigstatus provided. + (if (plist-member part :sigstatus) + (let ((from (notmuch-show-get-header :From msg)) + (sigstatus (car (plist-get part :sigstatus + (notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-header) + (notmuch-crypto-insert-sigstatus-button sigstatus from)) + +;; If we're not adding sigstatus, tell the user how to enable it. +(notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-header +'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -482,19 +484,21 @@ current buffer, if possible." t) (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add encryption status button if encstatus specified -(if (plist-member part :encstatus) - (let ((encstatus (car (plist-get part :encstatus - (notmuch-crypto-insert-encstatus-button encstatus) - ;; add signature status button if sigstatus specified - (if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) -(sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from - ;; if we're not adding encstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))) + ;; Add encryption status button if encstatus provided. + (if (plist-member part :encstatus) + (let ((encstatus (car (plist-get part :encstatus + (notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-header) +
[RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
On Mon, 23 Jan 2012 15:43:08 +0100, Florian Friesdorf wrote: > With notmuch-0.11 (require 'gnus-art') "fixes" the void > gnus-inhibit-images error. This is true, though as Aaron Ecay pointed out in the discussion of this patch, it does bring in all of gnus. My current workaround to the bug of gnus-inhibit-images being void is to load gnus, so that's just as bad, but it would be great if we didn't have to do that. > emacs git master has a fix for that, so you seem to have got through. I got a notification that it went in the gnus git repo a couple of weeks ago, so it is nice to know that it has been propagated to the emacs repo as well. Cheers, Chris
[PATCH v3 6/6] NEWS: update "Tag exclusion" section
Series LGTM. Quoth Pieter Praet on Jan 23 at 6:41 am: > --- > > Added note re new configurations, as suggested by Austin: > id:"20120123044108.GS16740 at mit.edu" > > NEWS | 19 +++ > 1 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/NEWS b/NEWS > index 2c2d9e9..2acdce5 100644 > --- a/NEWS > +++ b/NEWS > @@ -13,10 +13,21 @@ Reply to sender > > Tag exclusion > > - Tags can be automatically excluded from search results unless they > - appear explicitly in a query. By default, notmuch excludes the tags > - deleted and spam. This can be changed using the new config setting > - search.auto_exclude_tags. > + Tags can be automatically excluded from search results by adding them > + to the new 'search.exclude_tags' option in the Notmuch config file. > + > + This behaviour can be overridden by explicitly including an excluded > + tag in your query, for example: > + > +notmuch search $your_query and tag:$excluded_tag > + > + Existing users will probably want to run "notmuch setup" again to add > + the new well-commented [search] section to the configuration file. > + > + For new configurations, accepting the default setting will cause the > + tags "deleted" and "spam" to be excluded, equivalent to running: > + > +notmuch config set search.exclude_tags deleted spam > > Emacs Interface > ---
[PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
On Mon, 23 Jan 2012 11:02:04 +, David Edmondson wrote: > On Mon, 23 Jan 2012 00:34:23 -0800, Jameson Graef Rollins finestructure.net> wrote: > > -(defun notmuch-show-next-open-message () > > +(defun notmuch-show-next-open-message (&optional pop-at-end) > >"Show the next message." > > - (interactive) > > - (let (r) > > + (interactive "P") > > + (let ((parent-buffer notmuch-show-parent-buffer) > > + (r)) > > Extra brackets around 'r' are unnecessary. Otherwise, +1. This seems minor enough that I'm not going to resend, but noted for next time. Thanks. 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/20120123/da5ed563/attachment-0001.pgp>
[PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
On Mon, 23 Jan 2012 00:52:26 -0800, Jameson Graef Rollins wrote: > On Mon, 23 Jan 2012 08:16:03 +, David Edmondson wrote: > > There was no problem with the logic. The code in the two functions was > > almost identical, so I'd like to make any future changes in just one > > place. > > > > You didn't actually answer my question - is the logic in the new > > function correct? > > Honestly I didn't look too closely yet since I'm not convinced we need > the change at all. I would prefer to keep the functions separate. In > my opinion, enough special casing would be required that it wouldn't be > worth it, and it would make the code less clear. Okay. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/9dd6aeb7/attachment-0001.pgp>
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Mon, 23 Jan 2012 07:22:25 +, Jani Nikula wrote: > On Mon, 23 Jan 2012 06:05:27 +0100, Pieter Praet wrote: > > On Sun, 22 Jan 2012 14:53:41 -0800, Jameson Graef Rollins > finestructure.net> wrote: > > > On Sun, 22 Jan 2012 23:14:13 +0100, Xavier Maillard > > maillard.im> wrote: > > > > > > > > On Thu, 19 Jan 2012 20:19:03 +0100, Pieter Praet > > > > wrote: > > > > > If the 'search.exclude_tags' option is missing from the config file, > > > > > its value is automatically set to "deleted;spam;". Taking PoLS/DWIM > > > > > into account, this should probably only happen during setup. > > > > > > > > > > This patch is actually Austin Clements' work: > > > > > id:"20120117203211.GQ16740 at mit.edu" > > > > > > > > I do not think this is a sane default. As I told it in another post. I > > > > do not expect notmuch to skew my search queries not that I specifically > > > > asked. > > > > > > Hi, Xavier. Do you currently mark things as "deleted" or "spam"? If > > > not, this would have no affect on your search results. If you do, do > > > you currently expect those messages to show up in searches? If so, why > > > did you mark them as "deleted" or "spam" to begin with? > > > > > > I agree with your point in principle (ie. I don't generally want my > > > searches tampered with behind the scenes) but the issue here is about > > > messages that have been explicitly tagged as a form of "trash". Trash > > > is by it's nature something you're trying to get rid of. If you wanted > > > to find something in the future, why would you put it in the trash in > > > the first place? > > > > > > > You definitely have a point, but then again, who are we to assume that > > the terms "deleted" and "spam" have the *exact* same meaning for > > everyone? (also see id:"8739bbo0br.fsf at praet.org") > > "deleted" used to be a tag recognized by notmuch, and it used to sync to > the T (trashed) maildir flag. Even if notmuch won't delete any of your > mails now, I don't think you should use "deleted" on messages you want > to see again. Please let's not split hairs about this. > Agreed, but it might be nice to make a clear distinction between concepts and the actual tags mapped to them. I'm not suggestion we redefine the term "deleted", but from an internationalization standpoint, we shouldn't prevent users from mapping e.g. "verwijderd", "supprim?", "gel?scht", ... to the concept "deleted". > There really should be a definitive list of tags that are special to > lib/cli/emacs (like "inbox", "unread", "deleted", ...), or are > recommended for specific purposes (like "new" as an intermediate tag > before more sophisticated tagging), to avoid prolonged discussions like > this. > A list of recommended tags would definitely be nice, as long as they remain recommendations (as opposed to obligations), especially since there's really no reason to designate certain tags as being "special". > BR, > Jani. Peace -- Pieter
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Sun, 22 Jan 2012 21:34:00 -0800, Jameson Graef Rollins wrote: > On Mon, 23 Jan 2012 06:05:27 +0100, Pieter Praet wrote: > > You definitely have a point, but then again, who are we to assume that > > the terms "deleted" and "spam" have the *exact* same meaning for > > everyone? (also see id:"8739bbo0br.fsf at praet.org") > > Hrm. I'm not sure I buy this. Words already have meanings. If we're > going to start down a rabbit hole where we have to assume that users are > making up crazy alternate meanings for words, we're going to run into a > lot of problems. > True, but we're talking about tags here. Flexibility is their raison d'?tre. If we're going to impose semi-arbitrary limitations, we do away with alot of the benefits we were looking for in the first place. > Notmuch, or at least the emacs interface, already assumes a specific > meaning for certain terms, like most notably "inbox". Given that we're > dealing with english here, I think we have to assume common usage > meanings for any of the words we're using to describe anything. > Actually, we're only a small step away from being free to use whatever tag(s) we want for this. The term "inbox" is hardcoded in only 4 places, all of which are trivial to fix: - @ binary - `notmuch_config_open', where it's only used as a fallback when 'new.tags' isn't set (and reused to suggest a default value for 'new.tags' when running setup). - @ Emacs UI - `notmuch-saved-searches' (the function), where it's only used as a fallback when `notmuch-saved-searches' (the var) isn't set. - `notmuch-search-archive-thread' - `notmuch-show-archive-thread-internal' > This argument breaks a little in regards to "delete" since we're not > actually deleting anything in the sense of rm'ing it form the > filesystem, so we're already changing the meaning a bit. But see below. > [...] > > IMHO, this is one of those options that should remain disabled until > > *explicitly* set by the user. > > Ok, but then we're back to the incredibly prolonged discussion we've > been having about adding "delete" keys. If we disable this by default, > but add "delete" keys, the user might be in for a different surprise if > "deleted" messages keep showing up in searches. > > Basically we have two options as I see it: > > - add keys bindings to add "deleted" tags, and then *also* exclude > "tag:deleted" by default. > > - don't exclude anything by default, but then don't add any special keys > to handle "deleted" tags. > Adding a key to "delete" messages doesn't necessarily have to mean that the tag it adds should be hardcoded to "deleted". For example, our config file could contain something like this: #+begin_src conf [new] tags=unread;inbox; [tag] deleted=deleted;-inbox; archive=-inbox; [search] exclude_tags=deleted;spam; #+end_src (where all entries under the [tag] section could be used as aliases for "complex" tagging operations) ... and then we could "delete" messages using something like: #+begin_src emacs-lisp (define-key notmuch-show-mode-map "d" (lambda() (interactive) (notmuch-show-mod-tags (split-string (notmuch-config-get "tag.deleted") "\n")) (notmuch-show-next-open-message))) #+end_src (`notmuch-show-mod-tags' doesn't exist, of course, but see `notmuch-message-mark-replied' for a good example of how it could be work...) > There seemed to be a consensus forming that we in fact did want to add > the "deleted" key bindings. [...] +1. > [...] If we do that, then I think we should > generate the config file to exclude "deleted" tags by default. > > jamie. > > PS: when I say "exclude tags by default" I actually mean that the > setting should be added to the config file upon (re)generation. Nothing > should be excluded if nothing is set in the config file. Exactly! This is actually the *only* reason I involved myself with this whole conversation in the first place :) Peace -- Pieter
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Mon, 23 Jan 2012 09:03:42 +0100, Pieter Praet wrote: > On Mon, 23 Jan 2012 07:22:25 +, Jani Nikula wrote: > > On Mon, 23 Jan 2012 06:05:27 +0100, Pieter Praet > > wrote: > > > On Sun, 22 Jan 2012 14:53:41 -0800, Jameson Graef Rollins > > finestructure.net> wrote: > > > > On Sun, 22 Jan 2012 23:14:13 +0100, Xavier Maillard > > > maillard.im> wrote: > > > > > > > > > > On Thu, 19 Jan 2012 20:19:03 +0100, Pieter Praet > > > > praet.org> wrote: > > > > > > If the 'search.exclude_tags' option is missing from the config file, > > > > > > its value is automatically set to "deleted;spam;". Taking PoLS/DWIM > > > > > > into account, this should probably only happen during setup. > > > > > > > > > > > > This patch is actually Austin Clements' work: > > > > > > id:"20120117203211.GQ16740 at mit.edu" > > > > > > > > > > I do not think this is a sane default. As I told it in another post. I > > > > > do not expect notmuch to skew my search queries not that I > > > > > specifically > > > > > asked. > > > > > > > > Hi, Xavier. Do you currently mark things as "deleted" or "spam"? If > > > > not, this would have no affect on your search results. If you do, do > > > > you currently expect those messages to show up in searches? If so, why > > > > did you mark them as "deleted" or "spam" to begin with? > > > > > > > > I agree with your point in principle (ie. I don't generally want my > > > > searches tampered with behind the scenes) but the issue here is about > > > > messages that have been explicitly tagged as a form of "trash". Trash > > > > is by it's nature something you're trying to get rid of. If you wanted > > > > to find something in the future, why would you put it in the trash in > > > > the first place? > > > > > > > > > > You definitely have a point, but then again, who are we to assume that > > > the terms "deleted" and "spam" have the *exact* same meaning for > > > everyone? (also see id:"8739bbo0br.fsf at praet.org") > > > > "deleted" used to be a tag recognized by notmuch, and it used to sync to > > the T (trashed) maildir flag. Even if notmuch won't delete any of your > > mails now, I don't think you should use "deleted" on messages you want > > to see again. Please let's not split hairs about this. > > > > Agreed, but it might be nice to make a clear distinction between > concepts and the actual tags mapped to them. I'm not suggestion we > redefine the term "deleted", but from an internationalization > standpoint, we shouldn't prevent users from mapping e.g. "verwijderd", > "supprim?", "gel?scht", ... to the concept "deleted". > > > There really should be a definitive list of tags that are special to > > lib/cli/emacs (like "inbox", "unread", "deleted", ...), or are > > recommended for specific purposes (like "new" as an intermediate tag > > before more sophisticated tagging), to avoid prolonged discussions like > > this. > > > > A list of recommended tags would definitely be nice, as long as they > remain recommendations (as opposed to obligations), especially since > there's really no reason to designate certain tags as being "special". Whether there's reason or not, certain tags are special, for a fact, and they are not just recommendations. Perhaps one day someone will contribute patches to make them configurable, and separate the concepts from the actual tags, but in the mean time it will be easier to just document them for what they are. BR, Jani.
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Sun, 22 Jan 2012 23:38:40 -0800, Jameson Graef Rollins wrote: > On Mon, 23 Jan 2012 07:22:25 +, Jani Nikula wrote: > > There really should be a definitive list of tags that are special to > > lib/cli/emacs (like "inbox", "unread", "deleted", ...), or are > > recommended for specific purposes (like "new" as an intermediate tag > > before more sophisticated tagging), to avoid prolonged discussions like > > this. > > Just to be clear: the lib doesn't assign any special meaning to any tag > (as it shouldn't). The cli does, but only in the sense that it creates > config files that designate certain tags for certain operations by > default. It's really in emacs where certain tags currently have > unconfigurable meanings ("inbox"). The lib *does* assign special meaning to the tags it syncs to maildir flags: draft, flagged, passed, replied, unread. (deleted used to be part of the list.) The cli does have to request the syncing, but the mapping is in the lib (flag2tag array in lib/message.cc). BR, Jani.
[PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
On Sun, 22 Jan 2012 13:38:09 -0800, Jameson Graef Rollins wrote: > This patch seems to include multiple distinct changes. There is a > change to notmuch-show-insert-part-header, but a seemingly unrelated > change to the insertion of signed/encrypted part buttons. They should > be in separate patches. I can separate them. > I'm also not sure I understand why the proposed changes to the > signed/encrypted button insertion functions are necessary or desired. > Was there a problem with the logic as it was? What is gained by > having one function filled with special casing to handle two things, > rather than having two distinct functions? There was no problem with the logic. The code in the two functions was almost identical, so I'd like to make any future changes in just one place. You didn't actually answer my question - is the logic in the new function correct? > Finally, this patch throws out all the changes from the previous patch, > making the previous patch superfluous. I'll merge the first patch into the later (and presumably get accused of submitting patches which include multiple distinct changes :-)). -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/417cb6eb/attachment.pgp>
[PATCH] emacs: Make the part content available to `mm-inlinable-p'.
On Fri, 20 Jan 2012 12:04:27 -0800, Jameson Graef Rollins wrote: > On Thu, 19 Jan 2012 09:34:07 +, David Edmondson wrote: > > The `mm-inlinable-p' function works better if it has access to the > > data of the relevant part, so load that content before calling it. > > > > Don't load the content for parts that the user has indicated no desire > > to inline. > > So I'm a little confused here. Is there some variable I need to set to > get inlinable stuff to display inline? If so, I can't figure out what > it is. I don't see anything relevant in notmuch config, and I'm not > finding anything in mm- either, although I'm not quite sure what I'm > looking for. > > Currently I'm not getting any images displayed inline. I used to get > some, but not all, but at the moment I'm just getting a little white box > with no image in it. After applying this patch, I'm not getting any > attempt to inline images at all. But I'm assuming there is some > variable I need to set that I haven't... Are you talking about: - straightforward image/* attachments, - images in HTML which are part of a multipart/related ('cid:' images), - images in HTML which are referenced using HTTP (or FTP or ... I suppose). ? -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/978769ee/attachment-0001.pgp>
[PATCH v2] emacs: Make the part content available to the mm-inline* checks.
On Fri, 20 Jan 2012 12:13:49 -0800, Jameson Graef Rollins wrote: > On Wed, 18 Jan 2012 14:35:01 -0500, Austin Clements > wrote: > > Shouldn't we only be doing this for parts with inline (or not > > attachment) content-disposition? That's cheap to check. Or do we > > actually want things like image attachments to get inlined, despite > > their disposition? > > This is a good question, actually. Should we just always ignore the > disposition, and inline stuff if it's inlinable? Should this be > configurable? We shouldn't inline things unless the content disposition is 'inline'. I'll work on a fix for this, which will have nothing to do with whether or not things are indexed. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120123/c6322f92/attachment.pgp>
[PATCH] test/emacs-large-search-buffer: correct typo (EXPEXTED -> EXPECTED)
On Mon, 23 Jan 2012 05:26:10 +0100, Pieter Praet wrote: > introduced in commit 3b24b396 > --- > test/emacs-large-search-buffer | 12 ++-- > 1 files changed, 6 insertions(+), 6 deletions(-) pushed d
[PATCH] python: fix error handling
On Sun, 22 Jan 2012 14:09:35 +0100, Justus Winter <4winter at informatik.uni-hamburg.de> wrote: > Before 3434d1940 the return values of libnotmuch functions were > declared as c_void_p and the code checking for errors compared the > returned value to None, which is the ctypes equivalent of a NULL > pointer. Looks good to me as well. I pushed it to branch release. Could you make a NEWS patch against branch release (i.e. start 0.11.1) ? d
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Mon, 23 Jan 2012 06:05:27 +0100, Pieter Praet wrote: > On Sun, 22 Jan 2012 14:53:41 -0800, Jameson Graef Rollins finestructure.net> wrote: > > On Sun, 22 Jan 2012 23:14:13 +0100, Xavier Maillard > > wrote: > > > > > > On Thu, 19 Jan 2012 20:19:03 +0100, Pieter Praet > > > wrote: > > > > If the 'search.exclude_tags' option is missing from the config file, > > > > its value is automatically set to "deleted;spam;". Taking PoLS/DWIM > > > > into account, this should probably only happen during setup. > > > > > > > > This patch is actually Austin Clements' work: > > > > id:"20120117203211.GQ16740 at mit.edu" > > > > > > I do not think this is a sane default. As I told it in another post. I > > > do not expect notmuch to skew my search queries not that I specifically > > > asked. > > > > Hi, Xavier. Do you currently mark things as "deleted" or "spam"? If > > not, this would have no affect on your search results. If you do, do > > you currently expect those messages to show up in searches? If so, why > > did you mark them as "deleted" or "spam" to begin with? > > > > I agree with your point in principle (ie. I don't generally want my > > searches tampered with behind the scenes) but the issue here is about > > messages that have been explicitly tagged as a form of "trash". Trash > > is by it's nature something you're trying to get rid of. If you wanted > > to find something in the future, why would you put it in the trash in > > the first place? > > > > You definitely have a point, but then again, who are we to assume that > the terms "deleted" and "spam" have the *exact* same meaning for > everyone? (also see id:"8739bbo0br.fsf at praet.org") "deleted" used to be a tag recognized by notmuch, and it used to sync to the T (trashed) maildir flag. Even if notmuch won't delete any of your mails now, I don't think you should use "deleted" on messages you want to see again. Please let's not split hairs about this. There really should be a definitive list of tags that are special to lib/cli/emacs (like "inbox", "unread", "deleted", ...), or are recommended for specific purposes (like "new" as an intermediate tag before more sophisticated tagging), to avoid prolonged discussions like this. BR, Jani.
[PATCH v3 4/6] setup: Create functions for tag list printing and parsing
From: Austin Clements This refactors the tag list printing and parsing currently used for new.tags so that both can be reused for the new search.exclude_tags option. --- notmuch-setup.c | 55 ++- 1 files changed, 34 insertions(+), 21 deletions(-) diff --git a/notmuch-setup.c b/notmuch-setup.c index c3ea937..f85e0eb 100644 --- a/notmuch-setup.c +++ b/notmuch-setup.c @@ -87,6 +87,38 @@ welcome_message_post_setup (void) "have sufficient storage space available now.\n\n"); } +static void +print_tag_list (const char **tags, size_t tags_len) +{ +unsigned int i; +for (i = 0; i < tags_len; i++) { + if (i != 0) + printf (" "); + printf ("%s", tags[i]); +} +} + +static GPtrArray * +parse_tag_list (void *ctx, char *response) +{ +GPtrArray *tags = g_ptr_array_new (); +char *tag = response; +char *space; + +while (tag && *tag) { + space = strchr (tag, ' '); + if (space) + g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag)); + else + g_ptr_array_add (tags, talloc_strdup (ctx, tag)); + tag = space; + while (tag && *tag == ' ') + tag++; +} + +return tags; +} + int notmuch_setup_command (unused (void *ctx), unused (int argc), unused (char *argv[])) @@ -164,30 +196,11 @@ notmuch_setup_command (unused (void *ctx), new_tags = notmuch_config_get_new_tags (config, &new_tags_len); printf ("Tags to apply to all new messages (separated by spaces) ["); - -for (i = 0; i < new_tags_len; i++) { - if (i != 0) - printf (" "); - printf ("%s", new_tags[i]); -} - +print_tag_list (new_tags, new_tags_len); prompt ("]: "); if (strlen (response)) { - GPtrArray *tags = g_ptr_array_new (); - char *tag = response; - char *space; - - while (tag && *tag) { - space = strchr (tag, ' '); - if (space) - g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag)); - else - g_ptr_array_add (tags, talloc_strdup (ctx, tag)); - tag = space; - while (tag && *tag == ' ') - tag++; - } + GPtrArray *tags = parse_tag_list (ctx, response); notmuch_config_set_new_tags (config, (const char **) tags->pdata, tags->len); -- 1.7.8.1
[PATCH v3 6/6] NEWS: update "Tag exclusion" section
--- Added note re new configurations, as suggested by Austin: id:"20120123044108.GS16740 at mit.edu" NEWS | 19 +++ 1 files changed, 15 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 2c2d9e9..2acdce5 100644 --- a/NEWS +++ b/NEWS @@ -13,10 +13,21 @@ Reply to sender Tag exclusion - Tags can be automatically excluded from search results unless they - appear explicitly in a query. By default, notmuch excludes the tags - deleted and spam. This can be changed using the new config setting - search.auto_exclude_tags. + Tags can be automatically excluded from search results by adding them + to the new 'search.exclude_tags' option in the Notmuch config file. + + This behaviour can be overridden by explicitly including an excluded + tag in your query, for example: + +notmuch search $your_query and tag:$excluded_tag + + Existing users will probably want to run "notmuch setup" again to add + the new well-commented [search] section to the configuration file. + + For new configurations, accepting the default setting will cause the + tags "deleted" and "spam" to be excluded, equivalent to running: + +notmuch config set search.exclude_tags deleted spam Emacs Interface --- -- 1.7.8.1
[PATCH v3 5/6] setup: prompt user for search.exclude_tags value
Allow users to customize the search.exclude_tags option during setup. --- v2: - less copy-paste coding... :) v3: Austin's corrections [1] - @ `print_tag_list': add space before paren. - @ `notmuch_config_set_search_exclude_tags': remove \n between type cast and value. [1] id:"20120123043435.GR16740 at mit.edu" notmuch-setup.c | 20 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/notmuch-setup.c b/notmuch-setup.c index dcfa607..2941c52 100644 --- a/notmuch-setup.c +++ b/notmuch-setup.c @@ -133,6 +133,8 @@ notmuch_setup_command (unused (void *ctx), int is_new; const char **new_tags; size_t new_tags_len; +const char **search_exclude_tags; +size_t search_exclude_tags_len; #define prompt(format, ...)\ do { \ @@ -208,6 +210,24 @@ notmuch_setup_command (unused (void *ctx), g_ptr_array_free (tags, TRUE); } + +search_exclude_tags = notmuch_config_get_search_exclude_tags (config, &search_exclude_tags_len); + +printf ("Tags to exclude when searching messages (separated by spaces) ["); +print_tag_list (search_exclude_tags, search_exclude_tags_len); +prompt ("]: "); + +if (strlen (response)) { + GPtrArray *tags = parse_tag_list (ctx, response); + + notmuch_config_set_search_exclude_tags (config, + (const char **) tags->pdata, + tags->len); + + g_ptr_array_free (tags, TRUE); +} + + if (! notmuch_config_save (config)) { if (is_new) welcome_message_post_setup (); -- 1.7.8.1
Re: Infinite loop in emacs interface
On Tue, 17 Jan 2012 23:44:40 +0100, Rodney Lorrimar wrote: > > Debugger entered--Lisp error: (void-variable gnus-inhibit-images) > mm-shr((# ("text/html") nil nil nil nil nil nil)) > mm-inline-text-html((# ("text/html") nil nil nil nil nil > nil)) > mm-display-inline((# ("text/html") nil nil nil nil nil nil)) > mm-display-part((# ("text/html") nil nil nil nil nil nil)) > notmuch-show-mm-display-part-inline(...snipped...) > notmuch-search-show-thread(nil) > call-interactively(notmuch-search-show-thread nil nil) With emacs from git master as of today (1f48de2bbf78c87c1dee6425181b31be60d39c7c) and notmuch-0.11 you can workaround it: (require 'gnus-art) With emacs pretest 24.0.92 there is a bug in gnus. Fixes/workarounds are discussed in: "[PATCH] emacs: Make the part content available to `mm-inline* checks" -- Florian Friesdorf GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083 Jabber/XMPP: f...@chaoflow.net IRC: chaoflow on freenode,ircnet,blafasel,OFTC ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Sun, 22 Jan 2012 14:53:41 -0800, Jameson Graef Rollins wrote: > On Sun, 22 Jan 2012 23:14:13 +0100, Xavier Maillard > wrote: > > > > On Thu, 19 Jan 2012 20:19:03 +0100, Pieter Praet > > wrote: > > > If the 'search.exclude_tags' option is missing from the config file, > > > its value is automatically set to "deleted;spam;". Taking PoLS/DWIM > > > into account, this should probably only happen during setup. > > > > > > This patch is actually Austin Clements' work: > > > id:"20120117203211.GQ16740 at mit.edu" > > > > I do not think this is a sane default. As I told it in another post. I > > do not expect notmuch to skew my search queries not that I specifically > > asked. > > Hi, Xavier. Do you currently mark things as "deleted" or "spam"? If > not, this would have no affect on your search results. If you do, do > you currently expect those messages to show up in searches? If so, why > did you mark them as "deleted" or "spam" to begin with? > > I agree with your point in principle (ie. I don't generally want my > searches tampered with behind the scenes) but the issue here is about > messages that have been explicitly tagged as a form of "trash". Trash > is by it's nature something you're trying to get rid of. If you wanted > to find something in the future, why would you put it in the trash in > the first place? > You definitely have a point, but then again, who are we to assume that the terms "deleted" and "spam" have the *exact* same meaning for everyone? (also see id:"8739bbo0br.fsf at praet.org") IMHO, this is one of those options that should remain disabled until *explicitly* set by the user. > jamie. Peace -- Pieter
[PATCH] test/emacs-large-search-buffer: correct typo (EXPEXTED -> EXPECTED)
introduced in commit 3b24b396 --- test/emacs-large-search-buffer | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer index 6095e9d..4351e33 100755 --- a/test/emacs-large-search-buffer +++ b/test/emacs-large-search-buffer @@ -19,25 +19,25 @@ done notmuch new > /dev/null test_begin_subtest "Ensure that emacs doesn't drop results" -notmuch search '*' > EXPEXTED -sed -i -e 's/^thread:[0-9a-f]* //' -e 's/;//' -e 's/xx*/[BLOB]/' EXPEXTED -echo 'End of search results.' >> EXPEXTED +notmuch search '*' > EXPECTED +sed -i -e 's/^thread:[0-9a-f]* //' -e 's/;//' -e 's/xx*/[BLOB]/' EXPECTED +echo 'End of search results.' >> EXPECTED test_emacs '(notmuch-search "*") (notmuch-test-wait) (test-output)' sed -i -e s', *, ,g' -e 's/xxx*/[BLOB]/g' OUTPUT -test_expect_equal_file OUTPUT EXPEXTED +test_expect_equal_file OUTPUT EXPECTED test_begin_subtest "Ensure that emacs doesn't drop error messages" test_emacs '(notmuch-search "--this-option-does-not-exist") (notmuch-test-wait) (test-output)' -cat
[PATCH v2 6/6] NEWS: update "Tag exclusion" section
--- NEWS | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 2c2d9e9..a0b7929 100644 --- a/NEWS +++ b/NEWS @@ -13,10 +13,16 @@ Reply to sender Tag exclusion - Tags can be automatically excluded from search results unless they - appear explicitly in a query. By default, notmuch excludes the tags - deleted and spam. This can be changed using the new config setting - search.auto_exclude_tags. + Tags can be automatically excluded from search results by adding them + to the new 'search.exclude_tags' option in the Notmuch config file. + + This behaviour can be overridden by explicitly including an excluded + tag in your query, for example: + +notmuch search $your_query and tag:$excluded_tag + + Existing users will probably want to run "notmuch setup" again to add + the new well-commented [search] section to the configuration file. Emacs Interface --- -- 1.7.8.1
[PATCH v2 5/6] setup: prompt user for search.exclude_tags value
Allow users to customize the search.exclude_tags option during setup. --- notmuch-setup.c | 21 + 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/notmuch-setup.c b/notmuch-setup.c index dcfa607..0d75adc 100644 --- a/notmuch-setup.c +++ b/notmuch-setup.c @@ -133,6 +133,8 @@ notmuch_setup_command (unused (void *ctx), int is_new; const char **new_tags; size_t new_tags_len; +const char **search_exclude_tags; +size_t search_exclude_tags_len; #define prompt(format, ...)\ do { \ @@ -208,6 +210,25 @@ notmuch_setup_command (unused (void *ctx), g_ptr_array_free (tags, TRUE); } + +search_exclude_tags = notmuch_config_get_search_exclude_tags (config, &search_exclude_tags_len); + +printf ("Tags to exclude when searching messages (separated by spaces) ["); +print_tag_list(search_exclude_tags, search_exclude_tags_len); +prompt ("]: "); + +if (strlen (response)) { + GPtrArray *tags = parse_tag_list (ctx, response); + + notmuch_config_set_search_exclude_tags (config, + (const char **) + tags->pdata, + tags->len); + + g_ptr_array_free (tags, TRUE); +} + + if (! notmuch_config_save (config)) { if (is_new) welcome_message_post_setup (); -- 1.7.8.1
[PATCH v2 4/6] setup: move tag printing and parsing into separate functions
From: Austin Clements * notmuch-setup.c (notmuch_setup_command): Break tag printing and response parsing out into separate functions called `print_tag_list' respectively `parse_tag_list', for reuse with the 'search.exclude_tags' option. --- notmuch-setup.c | 55 ++- 1 files changed, 34 insertions(+), 21 deletions(-) diff --git a/notmuch-setup.c b/notmuch-setup.c index c3ea937..dcfa607 100644 --- a/notmuch-setup.c +++ b/notmuch-setup.c @@ -87,6 +87,38 @@ welcome_message_post_setup (void) "have sufficient storage space available now.\n\n"); } +static void +print_tag_list (const char **tags, size_t tags_len) +{ +unsigned int i; +for (i = 0; i < tags_len; i++) { + if (i != 0) + printf (" "); + printf ("%s", tags[i]); +} +} + +static GPtrArray * +parse_tag_list (void *ctx, char *response) +{ +GPtrArray *tags = g_ptr_array_new (); +char *tag = response; +char *space; + +while (tag && *tag) { + space = strchr (tag, ' '); + if (space) + g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag)); + else + g_ptr_array_add (tags, talloc_strdup (ctx, tag)); + tag = space; + while (tag && *tag == ' ') + tag++; +} + +return tags; +} + int notmuch_setup_command (unused (void *ctx), unused (int argc), unused (char *argv[])) @@ -164,30 +196,11 @@ notmuch_setup_command (unused (void *ctx), new_tags = notmuch_config_get_new_tags (config, &new_tags_len); printf ("Tags to apply to all new messages (separated by spaces) ["); - -for (i = 0; i < new_tags_len; i++) { - if (i != 0) - printf (" "); - printf ("%s", new_tags[i]); -} - +print_tag_list(new_tags, new_tags_len); prompt ("]: "); if (strlen (response)) { - GPtrArray *tags = g_ptr_array_new (); - char *tag = response; - char *space; - - while (tag && *tag) { - space = strchr (tag, ' '); - if (space) - g_ptr_array_add (tags, talloc_strndup (ctx, tag, space - tag)); - else - g_ptr_array_add (tags, talloc_strdup (ctx, tag)); - tag = space; - while (tag && *tag == ' ') - tag++; - } + GPtrArray *tags = parse_tag_list (ctx, response); notmuch_config_set_new_tags (config, (const char **) tags->pdata, tags->len); -- 1.7.8.1
[PATCH v2 3/6] config: only exclude messages if 'search.exclude_tags' is explicitly set
Currently, the 'search.exclude_tags' option is automatically set to "deleted;spam;" if it's missing from the config file. This violates the Principle of Least Surprise, so *only* set 'search.exclude_tags' to "deleted;spam;" if we didn't find a configuration file at all. This patch is actually Austin Clements' work: id:"20120117203211.GQ16740 at mit.edu" --- notmuch-config.c |8 ++-- test/search |1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/notmuch-config.c b/notmuch-config.c index 39da888..0ded6d7 100644 --- a/notmuch-config.c +++ b/notmuch-config.c @@ -362,8 +362,12 @@ notmuch_config_open (void *ctx, } if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) { - const char *tags[] = { "deleted", "spam" }; - notmuch_config_set_search_exclude_tags (config, tags, 2); + if (is_new) { + const char *tags[] = { "deleted", "spam" }; + notmuch_config_set_search_exclude_tags (config, tags, 2); + } else { + notmuch_config_set_search_exclude_tags (config, NULL, 0); + } } error = NULL; diff --git a/test/search b/test/search index 99d94bd..414be35 100755 --- a/test/search +++ b/test/search @@ -149,7 +149,6 @@ test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; N thread:XXX 2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)" test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured" -test_subtest_known_broken notmuch config set search.exclude_tags output=$(notmuch search subject:deleted | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) -- 1.7.8.1
[PATCH v2 2/6] test: only exclude "deleted" messages from search if explicitly configured
Currently, the 'search.exclude_tags' option is automatically set to "deleted;spam;" if it's missing from the config file. This violates the Principle of Least Surprise, so update the tests to *only* expect the exclusion of messages which are tagged "deleted" if the 'search.exclude_tags' option is explicitly set *and* contains that tag. --- test/search |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/test/search b/test/search index bf965e7..99d94bd 100755 --- a/test/search +++ b/test/search @@ -130,6 +130,7 @@ output=$(notmuch search "b?d?" | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; utf8-message-body-subject (inbox unread)" test_begin_subtest "Exclude \"deleted\" messages from search" +notmuch config set search.exclude_tags = deleted generate_message '[subject]="Not deleted"' generate_message '[subject]="Deleted"' notmuch new > /dev/null @@ -147,4 +148,11 @@ output=$(notmuch search subject:deleted | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) thread:XXX 2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply (deleted inbox unread)" +test_begin_subtest "Don't exclude \"deleted\" messages from search if not configured" +test_subtest_known_broken +notmuch config set search.exclude_tags +output=$(notmuch search subject:deleted | notmuch_search_sanitize) +test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread) +thread:XXX 2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted inbox unread)" + test_done -- 1.7.8.1
[PATCH v2 1/6] search: rename auto_exclude_tags to {search, }exclude_tags
All other config-related functions and args include the section title in their name, so for the sake of consistency, mirror that. Also, the "auto"matic part is a given, so that was dropped. --- notmuch-client.h |4 ++-- notmuch-config.c | 28 ++-- notmuch-count.c | 12 ++-- notmuch-search.c | 12 ++-- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/notmuch-client.h b/notmuch-client.h index 9c1d383..f5414f6 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -252,10 +252,10 @@ notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, notmuch_bool_t synchronize_flags); const char ** -notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t *length); +notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length); void -notmuch_config_set_auto_exclude_tags (notmuch_config_t *config, +notmuch_config_set_search_exclude_tags (notmuch_config_t *config, const char *list[], size_t length); diff --git a/notmuch-config.c b/notmuch-config.c index 8dcfe86..39da888 100644 --- a/notmuch-config.c +++ b/notmuch-config.c @@ -89,7 +89,7 @@ static const char search_config_comment[] = "\n" " The following option is supported here:\n" "\n" -"\tauto_exclude_tags\n" +"\texclude_tags\n" "\t\tA ;-separated list of tags that will be excluded from\n" "\t\tsearch results by default. Using an excluded tag in a\n" "\t\tquery will override that exclusion.\n"; @@ -106,8 +106,8 @@ struct _notmuch_config { const char **new_tags; size_t new_tags_length; notmuch_bool_t maildir_synchronize_flags; -const char **auto_exclude_tags; -size_t auto_exclude_tags_length; +const char **search_exclude_tags; +size_t search_exclude_tags_length; }; static int @@ -265,8 +265,8 @@ notmuch_config_open (void *ctx, config->new_tags = NULL; config->new_tags_length = 0; config->maildir_synchronize_flags = TRUE; -config->auto_exclude_tags = NULL; -config->auto_exclude_tags_length = 0; +config->search_exclude_tags = NULL; +config->search_exclude_tags_length = 0; if (! g_key_file_load_from_file (config->key_file, config->filename, @@ -361,9 +361,9 @@ notmuch_config_open (void *ctx, notmuch_config_set_new_tags (config, tags, 2); } -if (notmuch_config_get_auto_exclude_tags (config, &tmp) == NULL) { +if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) { const char *tags[] = { "deleted", "spam" }; - notmuch_config_set_auto_exclude_tags (config, tags, 2); + notmuch_config_set_search_exclude_tags (config, tags, 2); } error = NULL; @@ -624,20 +624,20 @@ notmuch_config_set_new_tags (notmuch_config_t *config, } const char ** -notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t *length) +notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length) { -return _config_get_list (config, "search", "auto_exclude_tags", -&(config->auto_exclude_tags), -&(config->auto_exclude_tags_length), length); +return _config_get_list (config, "search", "exclude_tags", +&(config->search_exclude_tags), +&(config->search_exclude_tags_length), length); } void -notmuch_config_set_auto_exclude_tags (notmuch_config_t *config, +notmuch_config_set_search_exclude_tags (notmuch_config_t *config, const char *list[], size_t length) { -_config_set_list (config, "search", "auto_exclude_tags", list, length, - &(config->auto_exclude_tags)); +_config_set_list (config, "search", "exclude_tags", list, length, + &(config->search_exclude_tags)); } /* Given a configuration item of the form . return the diff --git a/notmuch-count.c b/notmuch-count.c index f77861e..63459fb 100644 --- a/notmuch-count.c +++ b/notmuch-count.c @@ -35,8 +35,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[]) char *query_str; int opt_index; int output = OUTPUT_MESSAGES; -const char **auto_exclude_tags; -size_t auto_exclude_tags_length; +const char **search_exclude_tags; +size_t search_exclude_tags_length; unsigned int i; notmuch_opt_desc_t options[] = { @@ -78,10 +78,10 @@ notmuch_count_command (void *ctx, int argc, char *argv[]) return 1; } -auto_exclude_tags = notmuch_config_get_auto_exclude_tags - (config, &auto_exclude_tags_length); -for (i = 0; i < auto_exclude_tags_length; i++) - notmuch_query_add_tag_exclude (query, auto_exclude_tags[i]); +search_exclude_tags = notmuch_config_get_search_exclude_tags
[PATCH 4/4] setup: prompt user for search.exclude_tags value
On Sun, 22 Jan 2012 12:08:15 -0500, Austin Clements wrote: > Quoth Pieter Praet on Jan 22 at 7:55 am: > > On Thu, 19 Jan 2012 23:19:02 -0500, Austin Clements > > wrote: > > > [...] > > > Hah, okay. How about this as an initial minor refactoring of the code > > > for new.tags? > > > [...] > > > > Great, thanks! > > > > > > Would the following commit message be satisfactory? : > > > > #+begin_quote > > setup: move tag printing and parsing into separate functions > > > > * notmuch-setup.c (notmuch_setup_command): > > Break tag printing and response parsing out into separate functions > > called `print_tag_list' respectively `parse_tag_list', for reuse > > with the 'search.exclude_tags' option. > > #+end_quote > > > > (if not, please suggest an alternative; it'll be your name at the top) > > Sure. Great! Updated patch series follows. Peace -- Pieter
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Sun, 22 Jan 2012 23:14:13 +0100, Xavier Maillard wrote: > > On Thu, 19 Jan 2012 20:19:03 +0100, Pieter Praet wrote: > > If the 'search.exclude_tags' option is missing from the config file, > > its value is automatically set to "deleted;spam;". Taking PoLS/DWIM > > into account, this should probably only happen during setup. > > > > This patch is actually Austin Clements' work: > > id:"20120117203211.GQ16740 at mit.edu" > > I do not think this is a sane default. As I told it in another post. I > do not expect notmuch to skew my search queries not that I specifically > asked. > I agree 100%. I've responded in more detail @ your previous reply: id:"87ty3nawbt.fsf at praet.org" > /Xavier Peace -- Pieter
[PATCH v3 2/2] search: Support automatic tag exclusions
On Sun, 22 Jan 2012 23:09:30 +0100, Xavier Maillard wrote: > Hey Pieter, > Hi! > On Thu, 19 Jan 2012 20:19:00 +0100, Pieter Praet wrote: > > Nice feature! I won't be using it myself, but I can imagine it being > > *very* useful for those who still feel the need to "delete" email :). > > Adding a 'deleted' tag does not mean there will be a delete/purge > process ;) (currently I got 5k messages with the tag deleted ;). > Very true, that's why I put quotes around "delete". Deleting email for real is *so* old-fashioned... :D ;) > > Nitpicking: > > > > [ ... ] > > > So I'd like to suggest replacing all occurences of "auto_exclude_tags" > > with "search_exclude_tags" (and simply "exclude_tags" in the args to > > `_config_get_list' and `_config_set_list', of course). > > +1 > > > Unfortunately, this would also partially invalidate your recent NEWS > > submission [2]. > > > > - If the 'search.exclude_tags' option is missing from the config file, > > its value is automatically set to "deleted;spam;", which probably isn't > > a sane default. Luckily, you've already provided the solution [3]. > > I am against doing something /unsafe/ in the user's back. If there is no > option set intentionnaly by the user, there is nothing notmuch should > do -i.e no exclusion - > Absolutely. Actually, that's *exactly* what I meant. I thought that would be pretty clear, but perhaps it wasn't. I've reworded some of the commit messages, and will pay more attention to it in the future. > > - To make new users aware of the config option's existence, we should > > prompt them to configure it during setup. > > +1 > > /Xavier Thanks for your input! Peace -- Pieter
[PATCH] Add a NEWS section for 0.11.1 and document the python error handling bugfix
--- NEWS | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/NEWS b/NEWS index bf21e64..3d2c2a8 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,17 @@ +Notmuch 0.11.1 (2012-mm-dd) +=== + +Bug-fix release. + + +Fix error handling in python bindings. + + The python bindings in 0.11 failed to detect NULL pointers being + returned from libnotmuch functions and thus failed to raise + exceptions to indicate the error condition. Any subsequent calls + into libnotmuch caused segmentation faults. + + Notmuch 0.11 (2012-01-13) = -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] test/emacs-large-search-buffer: correct typo (EXPEXTED -> EXPECTED)
On Mon, 23 Jan 2012 05:26:10 +0100, Pieter Praet wrote: > introduced in commit 3b24b396 > --- > test/emacs-large-search-buffer | 12 ++-- > 1 files changed, 6 insertions(+), 6 deletions(-) pushed d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.
On Thu, 19 Jan 2012 12:54:02 +, David Edmondson wrote: > Add a new test function to allow simpler testing of emacs > functionality. > > `test_emacs_expect_t' takes one argument - a list expression to > evaluate. The test passes if the expression returns `t', otherwise it > fails and the output is reported to the tester. I've dealt with all of the comments for this patchset that I received with one exception (declaring test 3 as failing in patch 3). Any other comments before I submit another version? pgpXKQubl3VyQ.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] python: fix error handling
On Sun, 22 Jan 2012 14:09:35 +0100, Justus Winter <4win...@informatik.uni-hamburg.de> wrote: > Before 3434d1940 the return values of libnotmuch functions were > declared as c_void_p and the code checking for errors compared the > returned value to None, which is the ctypes equivalent of a NULL > pointer. Looks good to me as well. I pushed it to branch release. Could you make a NEWS patch against branch release (i.e. start 0.11.1) ? d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
v2 of these patches need to be merged with Mark's part saving stuff. I've done this, but I'll hold off posting the merged version until this one is reviewed, unless anyone objects. pgpVYndJoTvv0.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 6/6] emacs: modify the default show-mode key bindings for archiving
+1. pgprc98UhV8FC.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
On Mon, 23 Jan 2012 00:34:23 -0800, Jameson Graef Rollins wrote: > -(defun notmuch-show-next-open-message () > +(defun notmuch-show-next-open-message (&optional pop-at-end) >"Show the next message." > - (interactive) > - (let (r) > + (interactive "P") > + (let ((parent-buffer notmuch-show-parent-buffer) > + (r)) Extra brackets around 'r' are unnecessary. Otherwise, +1. pgpWtGpA0vR4m.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/6] emacs: add message archiving functions
+1. pgpViJzHc5v89.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/6] emacs: break out thread navigation from notmuch-show-archive-thread
+1. pgpNuY5fVgFxs.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/6] emacs: break up notmuch-show-archive-thread-internal into two more generally useful functions
On Mon, 23 Jan 2012 00:34:20 -0800, Jameson Graef Rollins wrote: > Brake up notmuch-show-archive-thread-internal into two new functions: "Break", otherwise +1. pgpyDxMQ6giYZ.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3 v2] emacs: Don't insert a part header if it's the first part and text/*.
Previously this logic applied only to text/plain. Allow it for other text/* parts as well. --- This was not included in version 1 of the patch set. emacs/notmuch-show.el |9 - 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 39f35ed..55e4e34 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -300,7 +300,9 @@ CONTENT-TYPE parts." (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment &rest button-parameters) - (unless (notmuch-show-hidden-part-header content-type) + (unless (or (notmuch-show-hidden-part-header content-type) + (and (= nth 1) + (string-match "text/*" content-type))) (apply #'insert-button (concat "[ " (if name (concat name ": ") "") @@ -561,10 +563,7 @@ current buffer, if possible." (defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type) (let ((start (point))) -;; If this text/plain part is not the first part in the message, -;; insert a header to make this clear. -(if (> nth 1) - (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))) +(notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename)) (insert (notmuch-show-get-bodypart-content msg part nth)) (save-excursion (save-restriction -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3 v2] emacs: Optionally hide some part headers.
Add a regexp, `notmuch-show-part-headers-hidden' and if the content-type of a part matches, don't show the part header. --- emacs/notmuch-show.el | 41 +++-- 1 files changed, 27 insertions(+), 14 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 9144484..39f35ed 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -110,6 +110,12 @@ indentation." :group 'notmuch :type 'boolean) +(defcustom notmuch-show-part-headers-hidden nil + "Headers for parts whose content-type matches this regexp will +not be shown." + :group 'notmuch + :type 'regexp) + (defmacro with-current-notmuch-show-message (&rest body) "Evaluate body with current buffer set to the text of current message" `(save-excursion @@ -285,23 +291,30 @@ message at DEPTH in the current thread." 'follow-link t 'face 'message-mml) +(defun notmuch-show-hidden-part-header (content-type) + "Return non-nil if a part header should be hidden for +CONTENT-TYPE parts." + (and notmuch-show-part-headers-hidden + (string-match notmuch-show-part-headers-hidden content-type))) + (defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment &rest button-parameters) - (apply #'insert-button -(concat "[ " -(if name (concat name ": ") "") -declared-type -(if (not (string-equal declared-type content-type)) -(concat " (as " content-type ")") - "") -(or comment "") -" ]") -:type 'notmuch-show-part-button-type -:notmuch-part nth -:notmuch-filename name -button-parameters) - (insert "\n")) + (unless (notmuch-show-hidden-part-header content-type) +(apply #'insert-button + (concat "[ " + (if name (concat name ": ") "") + declared-type + (if (not (string-equal declared-type content-type)) + (concat " (as " content-type ")") +"") + (or comment "") + " ]") + :type 'notmuch-show-part-button-type + :notmuch-part nth + :notmuch-filename name + button-parameters) +(insert "\n"))) ;; Functions handling particular MIME parts. -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3 v2] emacs: Don't return the button from `notmuch-show-insert-part-header'.
Instead, allow the caller to specify some parameters for the button. Rework `notmuch-show-insert-part-multipart/signed' and `notmuch-show-insert-part-multipart/encrypted' accordingly. --- Removed the merge of multipart/signed and multipart/encrypted. emacs/notmuch-show.el | 84 +--- 1 files changed, 44 insertions(+), 40 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 03c1f6b..9144484 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -285,24 +285,23 @@ message at DEPTH in the current thread." 'follow-link t 'face 'message-mml) -(defun notmuch-show-insert-part-header (nth content-type declared-type &optional name comment) - (let ((button)) -(setq button - (insert-button - (concat "[ " - (if name (concat name ": ") "") - declared-type - (if (not (string-equal declared-type content-type)) - (concat " (as " content-type ")") -"") - (or comment "") - " ]") - :type 'notmuch-show-part-button-type - :notmuch-part nth - :notmuch-filename name)) -(insert "\n") -;; return button -button)) +(defun notmuch-show-insert-part-header (nth content-type declared-type + &optional name comment + &rest button-parameters) + (apply #'insert-button +(concat "[ " +(if name (concat name ": ") "") +declared-type +(if (not (string-equal declared-type content-type)) +(concat " (as " content-type ")") + "") +(or comment "") +" ]") +:type 'notmuch-show-part-button-type +:notmuch-part nth +:notmuch-filename name +button-parameters) + (insert "\n")) ;; Functions handling particular MIME parts. @@ -460,15 +459,18 @@ current buffer, if possible." t) (defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add signature status button if sigstatus provided -(if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) - (sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from)) - ;; if we're not adding sigstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))) + ;; Add signature status button if sigstatus provided. + (if (plist-member part :sigstatus) + (let ((from (notmuch-show-get-header :From msg)) + (sigstatus (car (plist-get part :sigstatus + (notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-header) + (notmuch-crypto-insert-sigstatus-button sigstatus from)) + +;; If we're not adding sigstatus, tell the user how to enable it. +(notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-header +'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")) (let ((inner-parts (plist-get part :content)) (start (point))) @@ -482,19 +484,21 @@ current buffer, if possible." t) (defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type) - (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil))) -(button-put button 'face 'notmuch-crypto-part-header) -;; add encryption status button if encstatus specified -(if (plist-member part :encstatus) - (let ((encstatus (car (plist-get part :encstatus - (notmuch-crypto-insert-encstatus-button encstatus) - ;; add signature status button if sigstatus specified - (if (plist-member part :sigstatus) - (let* ((from (notmuch-show-get-header :From msg)) -(sigstatus (car (plist-get part :sigstatus - (notmuch-crypto-insert-sigstatus-button sigstatus from - ;; if we're not adding encstatus, tell the user how they can get it - (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."))) + ;; Add encryption status button if encstatus provided. + (if (plist-member part :encstatus) + (let ((encstatus (car (plist-get part :encstatus + (notmuch-show-insert-part-header nth declared-type content-type nil nil +'face 'notmuch-crypto-part-head
Re: [PATCH] python: fix error handling
On Sun, 22 Jan 2012 14:09:35 +0100, Justus Winter <4win...@informatik.uni-hamburg.de> wrote: > Before 3434d1940 the return values of libnotmuch functions were > declared as c_void_p and the code checking for errors compared the > returned value to None, which is the ctypes equivalent of a NULL > pointer. > > But said commit wrapped all the data types in python classes and the > semantic changed in a subtle way. If a function returns NULL, the > wrapped python value is falsish, but no longer equal to None. > > Backported from master to 0.11. > --- LGTM. Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
ignore folders patch?
Hi, On 2012-01-22T13:32:12 EET, Fabio Zanini wrote: > Hi! > > This is my first message to the list. In 2010 a patch was developed that > enabled notmuch new to ignore certain subdirectories of the mail folder: > > http://notmuchmail.org/pipermail/notmuch/2010/003170.html > > However, I cannot find any reference to it in the git history, and it > does not seem to have been implemented. > > This feature seems to be relevant, what would you think of reviving it > and including it into the codebase? I could try to update the patch > myself. I don't think my patch made it either: http://notmuchmail.org/pipermail/notmuch/2010/001103.html which support ignoring directories by dropping a '.noindex' file in them. I've been happily using that with the "mu" program for years. Best wishes, Dirk. -- Dirk-Jan C. Binnema Helsinki, Finland e:djcb at djcbsoftware.nl w:www.djcbsoftware.nl pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C
[PATCH] Automatically exclude tags in notmuch-show
On Sun, 22 Jan 2012 13:16:09 -0500, Austin Clements wrote: > Quoth myself on Jan 20 at 12:18 pm: > > Quoth Mark Walters on Jan 20 at 12:10 am: > > > > > > Ok Having said this is trivial I have found a problem. What should > > > notmuch do if you do something like > > > > > > notmuch show id: > > > and that message is marked with a deleted tag? To be consistent with the > > > other cases (where a deleted message is in a matched thread) we might > > > want to return the message with the not-matched flag set (eg in > > > JSON). But my patch doesn't, as it never even sees the thread since it > > > doesn't match. > > > > > > Looking at notmuch-show.c I think we should not apply the exclude tags > > > to do_show_single, but usually should apply it to do_show. One solution > > > which is simple and is at least close to right would be to get do_show > > > to return the number of threads found. If this is zero then retry the > > > query without the excludes (possible setting the match_flag to zero on > > > each message since we know it does not match) > > > > > > This is not a completely correct solution as if you ask notmuch-show to > > > show more than one thread it might threads which only contain deleted > > > messages. > > > > > > I can't see other good possibilities without slowing down the normal > > > path a lot (eg find all threads that match the original query and then > > > apply the argument above). > > > > > > Any thoughts? > > > > Oh dear. > > > > Well, here's one idea. Instead of doing a single thread query in > > show, do a thread query without the exclusions and then a message > > query with the exclusions. Output all of the messages from the first > > query, but use the results of the second query to determine which > > messages are "matched". The same could be accomplished in the library > > somewhat more efficiently, but it's not obvious to me what the API > > would be. > > Here's a slightly crazier idea that's more library-invasive than the > original approach, but probably better in the long run. > > Have notmuch_query_search_* return everything and make exclusion a > message flag like NOTMUCH_MESSAGE_FLAG_MATCH. Tweak the definition of > "matched" to mean "matched and not excluded" (specifically, a message > would have the match flag or the excluded flag or neither, but not > both). Search would skip threads with zero matched messages and I > think show would Just Work. > > I can think of two ways to implement this. notmuch_query_search_* > could perform both the original query and the query with exclusions > and use the docid set from the second to compute the "excluded" > message flag. Alternatively, it could examine the tags of each > message directly to compute the flag. The latter is probably easier > to implement, but probably slower. > > Thoughts? I have now thought about this some more and think I understand your idea (and how it would work) rather better now. I would suggest one small change: the flags for the messages returned should be "independent": so a message can match the query or not, and it can be excluded or not, with all 4 combinations being possible. (The consumer of notmuch_query_search_* would extract the information it wanted.) I have thought about some implementation ideas but I think sorting is going to be the deciding factor: what order should notmuch_query_search_* return messages/threads? For notmuch_query_search_messages either it returns them all together with the excluded messages marked, or returns all included ones, and then all excluded one. For notmuch_query_search_threads it is less clear. Currently it returns threads in order of first matching message. It is not clear what matching means now: is matching and included, or just matching? If the former then we will be returning some threads with no matching and included messages so we need to decide where to put them in the order. If we sort in both cases just on matching then we have the same output/sort as notmuch pre-excluded flags, just the frontends notmuch-search/show can decide to omit some lines/results. Note that after omitting "excluded" lines the thread sort would be different from the current notmuch-with-excluded implementation. Whereas if we sort based on matching and included, we keep the current sort order with some stuff appended. As regards implementation I think notmuch_query_search_messages is the crucial place: once that returns one of its two orders the rest sort of takes care of itself. Best wishes Mark
Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
On Mon, 23 Jan 2012 00:52:26 -0800, Jameson Graef Rollins wrote: > On Mon, 23 Jan 2012 08:16:03 +, David Edmondson wrote: > > There was no problem with the logic. The code in the two functions was > > almost identical, so I'd like to make any future changes in just one > > place. > > > > You didn't actually answer my question - is the logic in the new > > function correct? > > Honestly I didn't look too closely yet since I'm not convinced we need > the change at all. I would prefer to keep the functions separate. In > my opinion, enough special casing would be required that it wouldn't be > worth it, and it would make the code less clear. Okay. pgpUP0LNjefwn.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
On Mon, 23 Jan 2012 08:16:03 +, David Edmondson wrote: > There was no problem with the logic. The code in the two functions was > almost identical, so I'd like to make any future changes in just one > place. > > You didn't actually answer my question - is the logic in the new > function correct? Honestly I didn't look too closely yet since I'm not convinced we need the change at all. I would prefer to keep the functions separate. In my opinion, enough special casing would be required that it wouldn't be worth it, and it would make the code less clear. > I'll merge the first patch into the later (and presumably get accused of > submitting patches which include multiple distinct changes :-)). But if you're removing all the code anyway, it's not a distinct change. It's still just a replacement. jamie. pgpU0vPVJ8xBD.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3] emacs: Don't return the button from `notmuch-show-insert-part-header'.
On Mon, 23 Jan 2012 08:16:03 +, David Edmondson wrote: > There was no problem with the logic. The code in the two functions was > almost identical, so I'd like to make any future changes in just one > place. > > You didn't actually answer my question - is the logic in the new > function correct? Honestly I didn't look too closely yet since I'm not convinced we need the change at all. I would prefer to keep the functions separate. In my opinion, enough special casing would be required that it wouldn't be worth it, and it would make the code less clear. > I'll merge the first patch into the later (and presumably get accused of > submitting patches which include multiple distinct changes :-)). But if you're removing all the code anyway, it's not a distinct change. It's still just a replacement. 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/20120123/167021ca/attachment.pgp>
Re: [PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Mon, 23 Jan 2012 08:24:44 +, Jani Nikula wrote: > The lib *does* assign special meaning to the tags it syncs to maildir > flags: draft, flagged, passed, replied, unread. (deleted used to be part > of the list.) The cli does have to request the syncing, but the mapping > is in the lib (flag2tag array in lib/message.cc). Ah, you're absolutely right. My bad. jamie. pgpcs7F7RR3AU.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] config: only set search.exclude_tags to "deleted; spam; " during setup
On Mon, 23 Jan 2012 08:24:44 +, Jani Nikula wrote: > The lib *does* assign special meaning to the tags it syncs to maildir > flags: draft, flagged, passed, replied, unread. (deleted used to be part > of the list.) The cli does have to request the syncing, but the mapping > is in the lib (flag2tag array in lib/message.cc). Ah, you're absolutely right. My bad. 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/20120123/9242aa06/attachment.pgp>
[PATCH 6/6] emacs: modify the default show-mode key bindings for archiving
This changes the default key bindings for the 'a' key in notmuch-show mode. Instead of archiving the entire thread, it now just archives the current message, and then advance to the next open message (archive-message-then-next). 'A' is now bound to the previous archive-thread-then-next function. --- emacs/notmuch-show.el |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 8c71d03..29d9d92 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1043,7 +1043,8 @@ thread id. If a prefix is given, crypto processing is toggled." (define-key map "-" 'notmuch-show-remove-tag) (define-key map "+" 'notmuch-show-add-tag) (define-key map "x" 'notmuch-show-archive-thread-then-exit) - (define-key map "a" 'notmuch-show-archive-thread-then-next) + (define-key map "a" 'notmuch-show-archive-message-then-next) + (define-key map "A" 'notmuch-show-archive-thread-then-next) (define-key map "N" 'notmuch-show-next-message) (define-key map "P" 'notmuch-show-previous-message) (define-key map "n" 'notmuch-show-next-open-message) -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/6] emacs: break out thread navigation from notmuch-show-archive-thread
This function is now just for archiving the current thread. A new function is created to archive-then-next. The 'a' key binding is updated accordingly. This will allow people to bind to the simple thread archiving function without the extra navigation. The archive-thread function now also takes a prefix to unarchive the current thread (ie. put the whole thread back in the inbox). --- emacs/notmuch-show.el | 23 +-- 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 9638f35..21a655a 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1043,7 +1043,7 @@ thread id. If a prefix is given, crypto processing is toggled." (define-key map "-" 'notmuch-show-remove-tag) (define-key map "+" 'notmuch-show-add-tag) (define-key map "x" 'notmuch-show-archive-thread-then-exit) - (define-key map "a" 'notmuch-show-archive-thread) + (define-key map "a" 'notmuch-show-archive-thread-then-next) (define-key map "N" 'notmuch-show-next-message) (define-key map "P" 'notmuch-show-previous-message) (define-key map "n" 'notmuch-show-next-open-message) @@ -1303,7 +1303,7 @@ thread from the search from which this thread was originally shown." (interactive) (if (notmuch-show-advance) - (notmuch-show-archive-thread))) + (notmuch-show-archive-thread-then-next))) (defun notmuch-show-rewind () "Backup through the thread, (reverse scrolling compared to \\[notmuch-show-advance-and-archive]). @@ -1558,8 +1558,12 @@ added." (if show-next (notmuch-search-show-thread)) -(defun notmuch-show-archive-thread () - "Archive each message in thread, then show next thread from search. +(defun notmuch-show-archive-thread (&optional unarchive) + "Archive each message in thread. + +If a prefix argument is given, the messages will be +\"unarchived\" (ie. the \"inbox\" tag will be added instead of +removed). Archive each message currently shown by removing the \"inbox\" tag from each. Then kill this buffer and show the next thread @@ -1569,14 +1573,21 @@ Note: This command is safe from any race condition of new messages being delivered to the same thread. It does not archive the entire thread, but only the messages shown in the current buffer." + (interactive "P") + (if unarchive + (notmuch-show-add-tag-thread "inbox") +(notmuch-show-remove-tag-thread "inbox"))) + +(defun notmuch-show-archive-thread-then-next () + "Archive each message in thread, then show next thread from search." (interactive) - (notmuch-show-remove-tag-thread "inbox") + (notmuch-show-archive-thread) (notmuch-show-next-thread t)) (defun notmuch-show-archive-thread-then-exit () "Archive each message in thread, then exit back to search results." (interactive) - (notmuch-show-remove-tag-thread "inbox") + (notmuch-show-archive-thread) (notmuch-show-next-thread)) (defun notmuch-show-stash-cc () -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] emacs: add option to notmuch-show-next-open-message to pop out to parent buffer if at end
This will allow for keybindings that achieve a smoother message processing flow by reducing the number of key presses needed for most common operations. --- emacs/notmuch-show.el | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index fcd2878..fe7ec6a 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1373,17 +1373,23 @@ any effects from previous calls to (notmuch-show-mark-read) (notmuch-show-message-adjust)) -(defun notmuch-show-next-open-message () +(defun notmuch-show-next-open-message (&optional pop-at-end) "Show the next message." - (interactive) - (let (r) + (interactive "P") + (let ((parent-buffer notmuch-show-parent-buffer) + (r)) (while (and (setq r (notmuch-show-goto-message-next)) (not (notmuch-show-message-visible-p (if r (progn (notmuch-show-mark-read) (notmuch-show-message-adjust)) - (goto-char (point-max) + (if (and parent-buffer pop-at-end) + (progn + (kill-this-buffer) + (switch-to-buffer parent-buffer) + (notmuch-search-next-message)) + (goto-char (point-max)) (defun notmuch-show-previous-open-message () "Show the previous message." -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/6] emacs: use pop-at-end functionality in show-archive-message-then-next function
This provides a smoother message processing flow by reducing the number of key presses needed for these common operations. --- emacs/notmuch-show.el |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index fe7ec6a..8c71d03 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1611,7 +1611,7 @@ removed)." "Archive the current message, then show next thread from search." (interactive) (notmuch-show-archive-message) - (notmuch-show-next-open-message)) + (notmuch-show-next-open-message t)) (defun notmuch-show-stash-cc () "Copy CC field of current message to kill-ring." -- 1.7.8.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch