[PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Austin Clements
Quoth Dmitry Kurochkin on Nov 19 at  6:42 am:
> Hi Jamie.
> 
> On Fri, 18 Nov 2011 17:58:52 -0800, Jameson Graef Rollins  finestructure.net> wrote:
> > On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin  > gmail.com> wrote:
> > > Before the change, notmuch used g_mime_content_type_to_string(3)
> > > function to output Content-Type header value.  Turns out it outputs
> > > only "type/subtype" part and ignores all parameters.  Also, if there
> > > is no Content-Type header, default "text/plain" value is used.
> > 
> > Hi, Dmitry.  Can you explain under what circumstances you would need the
> > extra content-type parameters?
> 
> Charset is an example of a parameter which you need to render a part
> correctly.

Can notmuch convert to a common charset, given that, otherwise, every
client is going to have to implement this conversion anyway?

(And are there other examples of useful things in the content type?)


[PATCH] emacs: fix `mm-make-handle' content-type parameter

2011-11-18 Thread Dmitry Kurochkin
`notmuch-show-mm-display-part-inline' uses undocumented
`mm-make-handle' function.  One of the parameters for `mm-make-handle'
is charset.  Before the change, an unparsed content-type was given to
`mm-make-handle' (this was probably copied from Gnus source which uses
simple hard-coded values in a similar manner).  But in fact,
`mm-make-handle' expects a content-type value parsed by
`mail-header-parse-content-type'.  In particular, this is needed for
MIME display functions to correctly determine charset.
---
 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 d5c95d8..9fafafa 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -316,7 +316,8 @@ current buffer, if possible."
   (let ((display-buffer (current-buffer)))
 (with-temp-buffer
   (insert content)
-  (let ((handle (mm-make-handle (current-buffer) (list content-type
+  (let ((handle (mm-make-handle (current-buffer)
+   (mail-header-parse-content-type 
content-type
(set-buffer display-buffer)
(if (and (mm-inlinable-p handle)
 (mm-inlined-p handle))
-- 
1.7.7.3



Re: [PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Dmitry Kurochkin
On Fri, 18 Nov 2011 23:59:57 -0500, Austin Clements  wrote:
> Quoth Dmitry Kurochkin on Nov 19 at  6:42 am:
> > Hi Jamie.
> > 
> > On Fri, 18 Nov 2011 17:58:52 -0800, Jameson Graef Rollins 
> >  wrote:
> > > On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin 
> > >  wrote:
> > > > Before the change, notmuch used g_mime_content_type_to_string(3)
> > > > function to output Content-Type header value.  Turns out it outputs
> > > > only "type/subtype" part and ignores all parameters.  Also, if there
> > > > is no Content-Type header, default "text/plain" value is used.
> > > 
> > > Hi, Dmitry.  Can you explain under what circumstances you would need the
> > > extra content-type parameters?
> > 
> > Charset is an example of a parameter which you need to render a part
> > correctly.
> 
> Can notmuch convert to a common charset, given that, otherwise, every
> client is going to have to implement this conversion anyway?
> 

Notmuch can handle charset (and any other) parameters but only for known
media types (i.e. text/*).  I think it would be useful (especially for
human-readable output formats).  But it is a separate issue.

Notmuch can not convert other types it does not know how to handle.
E.g. HTML charset conversion is not as simple as for plain text.

AFAIK standard defines charset parameter just for few types.  So in
general, charset parameter can have any meaning for some custom media
type.

> (And are there other examples of useful things in the content type?)

What is meant by useful?  All parameters do have some use.  The fact
that notmuch does not handle them does not mean they are useless.  And
notmuch can not handle all parameters just because the list of
parameters is not defined.  So there is no choice but to let notmuch
users see and use these parameters.

Regards,
  Dmitry
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Austin Clements
Quoth Dmitry Kurochkin on Nov 19 at  6:42 am:
> Hi Jamie.
> 
> On Fri, 18 Nov 2011 17:58:52 -0800, Jameson Graef Rollins 
>  wrote:
> > On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin 
> >  wrote:
> > > Before the change, notmuch used g_mime_content_type_to_string(3)
> > > function to output Content-Type header value.  Turns out it outputs
> > > only "type/subtype" part and ignores all parameters.  Also, if there
> > > is no Content-Type header, default "text/plain" value is used.
> > 
> > Hi, Dmitry.  Can you explain under what circumstances you would need the
> > extra content-type parameters?
> 
> Charset is an example of a parameter which you need to render a part
> correctly.

Can notmuch convert to a common charset, given that, otherwise, every
client is going to have to implement this conversion anyway?

(And are there other examples of useful things in the content type?)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: fix sed error in basic tests

2011-11-18 Thread Dmitry Kurochkin
The error is easy to miss, because the test passes and stderr is not
printed.  But if you run basic tests in verbose mode (./basic
--verbose), you get:

  sed: can't read notmuch-test: No such file or directory

The issue is that sed command is given two files: notmuch-test and
$TEST_DIRECTORY/notmuch-test.  And there is no notmuch-test file in
the current directory (test/tmp.basic/).  The patch just removes the
non-existing file from the sed command.
---
 test/basic |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/basic b/test/basic
index 38db2ba..032c9f7 100755
--- a/test/basic
+++ b/test/basic
@@ -34,41 +34,41 @@ test_expect_success 'tests clean up after themselves' '
 '

 cleaner=no
 test_expect_code 1 'tests clean up even after a failure' '
 test_when_finished cleaner=yes &&
 (exit 1)
 '

 if test $clean$cleaner != yesyes
 then
say "bug in test framework: cleanup commands do not work reliably"
exit 1
 fi

 test_expect_code 2 'failure to clean up causes the test to fail' '
 test_when_finished "(exit 2)"
 '

 # Ensure that all tests are being run
 test_begin_subtest 'Ensure that all available tests will be run by 
notmuch-test'
-eval $(sed -n -e '/^TESTS="$/,/^"$/p' notmuch-test 
$TEST_DIRECTORY/notmuch-test)
+eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(ls -1 $TEST_DIRECTORY/ | \
 sed -r -e 
"/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
   -e 
"/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
   -e 
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d"
 \
   -e "/^(test.expected-output|.*~)/d" \
   -e "/^(gnupg-secret-key.asc)/d" \
   -e "/^(gnupg-secret-key.NOTE)/d" \
   -e "/^(atomicity.gdb)/d" \
   | sort)
 test_expect_equal "$tests_in_suite" "$available"

 EXPECTED=$TEST_DIRECTORY/test.expected-output
 suppress_diff_date() {
 sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
-e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
 }

 test_begin_subtest "Ensure that test output is suppressed unless the test 
fails"
 output=$(cd $TEST_DIRECTORY; ./test-verbose 2>&1 | suppress_diff_date)
-- 
1.7.7.3



Re: [PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Dmitry Kurochkin
Hi Jamie.

On Fri, 18 Nov 2011 17:58:52 -0800, Jameson Graef Rollins 
 wrote:
> On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin 
>  wrote:
> > Before the change, notmuch used g_mime_content_type_to_string(3)
> > function to output Content-Type header value.  Turns out it outputs
> > only "type/subtype" part and ignores all parameters.  Also, if there
> > is no Content-Type header, default "text/plain" value is used.
> 
> Hi, Dmitry.  Can you explain under what circumstances you would need the
> extra content-type parameters?

Charset is an example of a parameter which you need to render a part
correctly.

>  It just seems like a lot of extra noise
> in the output to me, but that's partially because I can't think of any
> reason why something that is receiving pre-parsed mime content would
> need it.  Maybe there's a better way to handle what you're trying to get
> to.
> 

Why extra output in JSON is an issue?

The parameters are there for a reason.  They are part of the
content-type and are needed to handle the body properly.  If you say
that the parameters are not needed by notmuch users, that implies that
they are handled by notmuch.  Which is just not possible.

> I think it would help a lot if you could submit some sort of test
> modification that demonstrates the issue.  This is one of the reasons we
> keep emphasizing that it's good to first have tests in hand that
> demonstrate issues before patches that address them.
> 

The fact that this change happens to fix an issue with HTML charsets for
me is just a side effect.

The real issue is that JSON format throws away information which is
required to properly render a part.  I do not think we need to add a
dedicated test to check that JSON outputs charsets with parameters,
considering that it is already present in many other tests.

I do not think it was intended that notmuch outputs stripped
Content-Type values.  It was just a side effect of using
g_mime_content_type_to_string(3) which gone unnoticed.

> >   "content": [{"id": 2,
> > - "content-type": "text/plain",
> >   "content": "This is a test signed message.\n"},
> 
> Without figuring out what's going on, I notice that some of the tests
> have been modified to remove the content-type fields on a bunch of
> parts.  I think that is probably not right.
> 

I tried to explain this in the preable.  These parts do not have
Content-Type in the original message.  So I think it is wrong for
notmuch JSON format to add it.

Regards,
  Dmitry

> jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Jameson Graef Rollins
On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin 
 wrote:
> Before the change, notmuch used g_mime_content_type_to_string(3)
> function to output Content-Type header value.  Turns out it outputs
> only "type/subtype" part and ignores all parameters.  Also, if there
> is no Content-Type header, default "text/plain" value is used.

Hi, Dmitry.  Can you explain under what circumstances you would need the
extra content-type parameters?  It just seems like a lot of extra noise
in the output to me, but that's partially because I can't think of any
reason why something that is receiving pre-parsed mime content would
need it.  Maybe there's a better way to handle what you're trying to get
to.

I think it would help a lot if you could submit some sort of test
modification that demonstrates the issue.  This is one of the reasons we
keep emphasizing that it's good to first have tests in hand that
demonstrate issues before patches that address them.

>   "content": [{"id": 2,
> - "content-type": "text/plain",
>   "content": "This is a test signed message.\n"},

Without figuring out what's going on, I notice that some of the tests
have been modified to remove the content-type fields on a bunch of
parts.  I think that is probably not right.

jamie.


pgpVWp48YSL8h.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] Output unmodified Content-Type header value for JSON format.

2011-11-18 Thread Jameson Graef Rollins
On Sat, 19 Nov 2011 03:45:05 +0400, Dmitry Kurochkin  wrote:
> Before the change, notmuch used g_mime_content_type_to_string(3)
> function to output Content-Type header value.  Turns out it outputs
> only "type/subtype" part and ignores all parameters.  Also, if there
> is no Content-Type header, default "text/plain" value is used.

Hi, Dmitry.  Can you explain under what circumstances you would need the
extra content-type parameters?  It just seems like a lot of extra noise
in the output to me, but that's partially because I can't think of any
reason why something that is receiving pre-parsed mime content would
need it.  Maybe there's a better way to handle what you're trying to get
to.

I think it would help a lot if you could submit some sort of test
modification that demonstrates the issue.  This is one of the reasons we
keep emphasizing that it's good to first have tests in hand that
demonstrate issues before patches that address them.

>   "content": [{"id": 2,
> - "content-type": "text/plain",
>   "content": "This is a test signed message.\n"},

Without figuring out what's going on, I notice that some of the tests
have been modified to remove the content-type fields on a bunch of
parts.  I think that is probably not right.

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/2018/bf0ebc6e/attachment.pgp>


Re: [PATCH] emacs: fix `mm-make-handle' content-type parameter

2011-11-18 Thread Dmitry Kurochkin
Please ignore this patch.  Apparently, it is not enough to fix the
issue.  A proper fix is available in [1].

Regards,
  Dmitry

[1] id:"1321659905-24367-1-git-send-email-dmitry.kuroch...@gmail.com"
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: make all tests terminable with Ctrl-c

2011-11-18 Thread David Bremner
On Tue, 08 Nov 2011 18:02:25 +0200, Tomi Ollila  wrote:
> Some tests don't break when HUP signal is sent tho those (by
> pressing ctrl-c on the terminal). Therefore, the top-level
> test script catches the HUP and sends TERM signal to the
> started test script.

Pushed to master

d


[PATCH] test: emacs: tidy up "Stashing in notmuch-show" test

2011-11-18 Thread David Bremner
On Wed, 16 Nov 2011 12:38:19 +0100, Pieter Praet  wrote:
> Merge expected output into the actual test, so we can verify the stashed
> filename using ${gen_msg_filename} instead of doing sed tricks.

pushed to master

d


[PATCH] emacs: create patch filename from subject for inline patch fake parts

2011-11-18 Thread Jani Nikula
Use the mail subject line for creating a descriptive filename for the wash
generated inline patch fake parts. The names are similar to the ones
created by 'git format-patch', just without the leading numbers.

Signed-off-by: Jani Nikula 

---

I know notmuch-subject-to-patch-filename is totally un-lispy. Suggestions
welcome on how to make it lispy and keep it somewhat readable.

If we later want to have a '>' counterpart to '|' to save messages to files
rather than pipe, then this could be generalized and re-used for creating
the suggested filename for that.

I don't even use the notmuch-wash-convert-inline-patch-to-part option that
much, but having it suggest "inline patch" as filename is just ugly...
---
 emacs/notmuch-wash.el |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 1f420b2..755d64a 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -290,6 +290,17 @@ When doing so, maintaining citation leaders in the wrapped 
text."
 
 (defvar diff-file-header-re) ; From `diff-mode.el'.
 
+(defun notmuch-subject-to-patch-filename (str)
+  "Convert a typical patch mail subject line into a suitable filename."
+  (let ((s str))
+(setq s (replace-regexp-in-string "^ *\\(\\[[^]]*\\]\\)? *" "" s))
+(setq s (replace-regexp-in-string "[. ]*$" "" s))
+(setq s (replace-regexp-in-string "[^A-Za-z0-9._-]+" "-" s))
+(setq s (replace-regexp-in-string "\\.+" "." s))
+(when (> (length s) 52)
+  (setq s (substring s 0 52)))
+(concat s ".patch")))
+
 (defun notmuch-wash-convert-inline-patch-to-part (msg depth)
   "Convert an inline patch into a fake 'text/x-diff' attachment.
 
@@ -316,7 +327,10 @@ for error."
(setq part (plist-put part :content-type "text/x-diff"))
(setq part (plist-put part :content (buffer-string)))
(setq part (plist-put part :id -1))
-   (setq part (plist-put part :filename "inline patch"))
+   (setq part (plist-put part :filename
+ (notmuch-subject-to-patch-filename
+  (plist-get
+   (plist-get msg :headers) :Subject
(delete-region (point-min) (point-max))
(notmuch-show-insert-bodypart nil part depth))
 
-- 
1.7.5.4

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: make all tests terminable with Ctrl-c

2011-11-18 Thread David Bremner
On Tue, 08 Nov 2011 18:02:25 +0200, Tomi Ollila  wrote:
> Some tests don't break when HUP signal is sent tho those (by
> pressing ctrl-c on the terminal). Therefore, the top-level
> test script catches the HUP and sends TERM signal to the
> started test script.

Pushed to master

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: emacs: tidy up "Stashing in notmuch-show" test

2011-11-18 Thread David Bremner
On Wed, 16 Nov 2011 12:38:19 +0100, Pieter Praet  wrote:
> Merge expected output into the actual test, so we can verify the stashed
> filename using ${gen_msg_filename} instead of doing sed tricks.

pushed to master

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v2] test: attempt to send QUIT to smtp-dummy in case mail send failed

2011-11-18 Thread David Bremner
On Wed, 16 Nov 2011 23:25:19 +0200, Tomi Ollila  wrote:
> If mail sending from emacs fails before it has chance to connect
> to the smtp-dummy mail server, the opportunistic QUIT message
> sending makes smtp-dummy to exit.

Pushed.

d


[PATCH 0/9] test: (hopefully) better test prerequisites

2011-11-18 Thread Tomi Ollila
On Thu, 17 Nov 2011 19:17:16 +0400, Dmitry Kurochkin  wrote:
> 
> If we follow this pattern than all code like:
> 
>   f() {
> if (!done_once)
> do_once
> 
> do_more
>   }
> 
> Should be rewritten using dynamic functions. I do not think I agree with
> this :)
> 
> Anyway, all above is just IMHO.  You should probably go ahead and
> prepare a patch implementing this approach for others to review.

I probably won't. While I was looking something in your patch and I was
thinking how to fix I just got this idea and wrote it to see whether
other's see as I do. The discussion got a bit side-tracked as I just
look this tiny part of the whole. Your later patch looks more
understandable than the previous (which emacs || emacs || return) and
it is something I can live with :) -- Just for now I'm not going to 
work on the whole anyway.

> 
> Regards,
>   Dmitry
> 

Thanks,
Tomi


[PATCH] emacs: fix `mm-make-handle' content-type parameter

2011-11-18 Thread Dmitry Kurochkin
`notmuch-show-mm-display-part-inline' uses undocumented
`mm-make-handle' function.  One of the parameters for `mm-make-handle'
is charset.  Before the change, an unparsed content-type was given to
`mm-make-handle' (this was probably copied from Gnus source which uses
simple hard-coded values in a similar manner).  But in fact,
`mm-make-handle' expects a content-type value parsed by
`mail-header-parse-content-type'.  In particular, this is needed for
MIME display functions to correctly determine charset.
---
 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 d5c95d8..9fafafa 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -316,7 +316,8 @@ current buffer, if possible."
   (let ((display-buffer (current-buffer)))
 (with-temp-buffer
   (insert content)
-  (let ((handle (mm-make-handle (current-buffer) (list content-type
+  (let ((handle (mm-make-handle (current-buffer)
+   (mail-header-parse-content-type 
content-type
(set-buffer display-buffer)
(if (and (mm-inlinable-p handle)
 (mm-inlined-p handle))
-- 
1.7.7.3

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: fix sed error in basic tests

2011-11-18 Thread Dmitry Kurochkin
The error is easy to miss, because the test passes and stderr is not
printed.  But if you run basic tests in verbose mode (./basic
--verbose), you get:

  sed: can't read notmuch-test: No such file or directory

The issue is that sed command is given two files: notmuch-test and
$TEST_DIRECTORY/notmuch-test.  And there is no notmuch-test file in
the current directory (test/tmp.basic/).  The patch just removes the
non-existing file from the sed command.
---
 test/basic |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/basic b/test/basic
index 38db2ba..032c9f7 100755
--- a/test/basic
+++ b/test/basic
@@ -34,41 +34,41 @@ test_expect_success 'tests clean up after themselves' '
 '
 
 cleaner=no
 test_expect_code 1 'tests clean up even after a failure' '
 test_when_finished cleaner=yes &&
 (exit 1)
 '
 
 if test $clean$cleaner != yesyes
 then
say "bug in test framework: cleanup commands do not work reliably"
exit 1
 fi
 
 test_expect_code 2 'failure to clean up causes the test to fail' '
 test_when_finished "(exit 2)"
 '
 
 # Ensure that all tests are being run
 test_begin_subtest 'Ensure that all available tests will be run by 
notmuch-test'
-eval $(sed -n -e '/^TESTS="$/,/^"$/p' notmuch-test 
$TEST_DIRECTORY/notmuch-test)
+eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(ls -1 $TEST_DIRECTORY/ | \
 sed -r -e 
"/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
   -e 
"/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
   -e 
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d"
 \
   -e "/^(test.expected-output|.*~)/d" \
   -e "/^(gnupg-secret-key.asc)/d" \
   -e "/^(gnupg-secret-key.NOTE)/d" \
   -e "/^(atomicity.gdb)/d" \
   | sort)
 test_expect_equal "$tests_in_suite" "$available"
 
 EXPECTED=$TEST_DIRECTORY/test.expected-output
 suppress_diff_date() {
 sed -e 's/\(.*\-\-\- test-verbose\.4\.\expected\).*/\1/' \
-e 's/\(.*\+\+\+ test-verbose\.4\.\output\).*/\1/'
 }
 
 test_begin_subtest "Ensure that test output is suppressed unless the test 
fails"
 output=$(cd $TEST_DIRECTORY; ./test-verbose 2>&1 | suppress_diff_date)
-- 
1.7.7.3

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] test: attempt to send QUIT to smtp-dummy in case mail send failed

2011-11-18 Thread David Bremner
On Wed, 16 Nov 2011 23:25:19 +0200, Tomi Ollila  wrote:
> If mail sending from emacs fails before it has chance to connect
> to the smtp-dummy mail server, the opportunistic QUIT message
> sending makes smtp-dummy to exit.

Pushed.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/9] test: (hopefully) better test prerequisites

2011-11-18 Thread Dmitry Kurochkin
On Thu, 17 Nov 2011 16:02:46 +0200, Tomi Ollila  wrote:
> On Thu, 17 Nov 2011 17:22:41 +0400, Dmitry Kurochkin 
>  wrote:
> > Hi Tomi.
> > 
> > On Thu, 17 Nov 2011 14:20:19 +0200, Tomi Ollila  wrote:
> > > 
> > > I.e. dynamically, at run-time, re-create test_emacs function...
> > > 
> > 
> > Could you please add some human-friendly description on what benefits
> > the code above has? :)
> 
> "All problems in computer science can be solved by another level of 
> indirection"
> 
> > The only change I see (besides code refactoring) is 30sec timeout (BTW
> > you can replace the list of numbers with "seq 30") instead of infinite
> > wait loop.  Which may be a good, but unrelated change.
> 
> Can't do `seq 30` it is not portable. BSD world uses different command.
> 
> > As for re-creating functions dynamically, I find the above code more
> > complex than the existing one.  I think the current code is more
> > shell-style and easier to understand.  Just IMHO.
> 
> Think it as a function pointer (I should have renamed that as test_emacs_p ;)
> 
> Shell is hyper-dynamic being (like python). New functionality can be
> written 'on-the-fly' inside functions (often without eval) Even variables 
> can be referenced as function names. Hmm, even perl4 has this way of 
> working supported...
> 
> IMHO it is simpler when one get's used to.
> 

I have no problem with thinking about it as a function pointer.  The
problem is that shell does not have function pointers :)

I am all for abstraction.  And I like lisp and haskell :) But it should
make things easier and more clear, not more complex.  IMO the old code
is easier to follow.  E.g. at any moment I understand what the function
would do, depending on the EMACS_SERVER value.  With functions being
dynamically overwritten, I have no idea what code would be executed when
the function is called.

Another point: EMACS_SERVER variable and the "function pointer" are kind
of duplicating each other.  And both have to be in sync which brings
possibility for new bugs.

If we follow this pattern than all code like:

  f() {
if (!done_once)
do_once

do_more
  }

Should be rewritten using dynamic functions. I do not think I agree with
this :)

Anyway, all above is just IMHO.  You should probably go ahead and
prepare a patch implementing this approach for others to review.

> ... just that the test "framework" needs some refactoring... along the
> way all of these "practises" should be defined. code style, variable names
> consistency, etc...  
> 

Yeah, there are some rough edges.  But it is just a test suite.  I care
more about core notmuch and emacs UI :)

> You've done good work with this 'prereq' -thing.

Thanks.

> It's just hard to see
> the elegance here.

Come on, it is sooo obvious :)

Regards,
  Dmitry

> I know this very well as I have to maintain my
> own 'evolutionary' developed code -- when priority is on short-term
> cost savings over code consistency..
> 
> > 
> > Regards,
> >   Dmitry
> > 
> > > 
> > > Tomi
> 
> Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


FYI: gmail contacts as notmuch emacs addressbook

2011-11-18 Thread Jani Nikula

This is a really simple read-only setup for using gmail contacts as
notmuch addressbook, but perhaps there are people who find this useful:

1) Install goobook http://code.google.com/p/goobook/. On a Debian based
system you might be able do it with:

$ sudo apt-get install python-pip
$ sudo pip install goobook

2) Setup goobook. I simply have 'machine google.com' line in my .netrc,
and no separate .goobookrc, but YMMV. See 'goobook --help' and 'goobook
config-template' for more.

3) Create (and improve! ;) script:

#!/bin/sh
goobook query "$*" | sed 's/\(.*\)\t\(.*\)\t.*/\2 \<\1\>/' | sed '/^$/d'

4) Point notmuch-address-command to the above script as explained in
http://notmuchmail.org/emacstips/#index11h2.

5) Enjoy tab completion to gmail contacts in emacs ui.


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 0/9] test: (hopefully) better test prerequisites

2011-11-18 Thread Tomi Ollila
On Thu, 17 Nov 2011 19:17:16 +0400, Dmitry Kurochkin 
 wrote:
> 
> If we follow this pattern than all code like:
> 
>   f() {
> if (!done_once)
> do_once
> 
> do_more
>   }
> 
> Should be rewritten using dynamic functions. I do not think I agree with
> this :)
> 
> Anyway, all above is just IMHO.  You should probably go ahead and
> prepare a patch implementing this approach for others to review.

I probably won't. While I was looking something in your patch and I was
thinking how to fix I just got this idea and wrote it to see whether
other's see as I do. The discussion got a bit side-tracked as I just
look this tiny part of the whole. Your later patch looks more
understandable than the previous (which emacs || emacs || return) and
it is something I can live with :) -- Just for now I'm not going to 
work on the whole anyway.

> 
> Regards,
>   Dmitry
> 

Thanks,
Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 3/3] RELEASING: document the semi-automated version propagation.

2011-11-18 Thread David Bremner
From: David Bremner 

The instructions are purposely a bit coy about what files are updated,
so we don't have to update immediately when something else is plugged
into the make recipe.
---
 RELEASING |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/RELEASING b/RELEASING
index e3e0cef..88dab04 100644
--- a/RELEASING
+++ b/RELEASING
@@ -62,11 +62,11 @@ repository. From here, there are just a few steps to 
release:
be "1.0.1" and a subsequent bug-fix release would be "1.0.2"
etc.
 
-   Update bindings/python/notmuch/version.py to match version.
+   When you are happy with the file 'version', run
 
-   Update the version in notmuch.1 to match version.
+make update-versions
 
-   XXX: Probably these last two steps should be (semi-)automated.
+   to propagate the version to the other places needed.
 
Commit these changes.
 
-- 
1.7.7.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[DRAFT PATCH] emacs: support limiting the number of results shown in search results

2011-11-18 Thread Jani Nikula
Add support for limiting the maximum number of results initially displayed
in search results. When enabled, the search results will contain push
buttons to double the number of results displayed or to show unlimited
results.

The approach is inspired by vc-print-log in Emacs vc.el.

Signed-off-by: Jani Nikula 

---

Note: This is not intended for inclusion yet. It works, but I'd like to try
to simplify things a bit (in this patch and otherwise) by keeping the
relevant variables buffer local over notmuch-search-refresh-view. This
would mean not killing the search buffer in between.

I just wanted to share the work-in-progress, rebased against the current
master and the merged lib/cli dependencies. The previous versions of the
emacs part is at:
id:"0b937eb1103dbb24142a783eff599a7cb08f195e.1320093940.git.j...@nikula.org"
---
 emacs/notmuch-hello.el |   18 +++--
 emacs/notmuch.el   |   62 +++
 2 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 1a76c30..0e165e7 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -26,7 +26,7 @@
 (require 'notmuch-lib)
 (require 'notmuch-mua)
 
-(declare-function notmuch-search "notmuch" (query &optional oldest-first 
target-thread target-line continuation))
+(declare-function notmuch-search "notmuch" (query &optional oldest-first limit 
target-thread target-line continuation))
 (declare-function notmuch-poll "notmuch" ())
 
 (defvar notmuch-hello-search-bar-marker nil
@@ -37,6 +37,18 @@
   :type 'integer
   :group 'notmuch)
 
+(defcustom notmuch-search-limit nil
+  "The maximum number of results to show in search results.
+
+This variables controls the maximum number of results to
+initially show in search results. If nil, the number of results
+is not limited. If non-nil, the search results will contain push
+buttons to double the number (can be repeated) or show unlimited
+number of results."
+  :type '(choice (const :tag "Unlimited" nil)
+(integer :tag "Limit"))
+  :group 'notmuch)
+
 (defcustom notmuch-show-empty-saved-searches nil
   "Should saved searches with no messages be listed?"
   :type 'boolean
@@ -151,7 +163,7 @@ Typically \",\" in the US and UK and \".\" in Europe."
 (defun notmuch-hello-search (search)
   (let ((search (notmuch-hello-trim search)))
 (notmuch-hello-remember-search search)
-(notmuch-search search notmuch-search-oldest-first nil nil 
#'notmuch-hello-search-continuation)))
+(notmuch-search search notmuch-search-oldest-first notmuch-search-limit 
nil nil #'notmuch-hello-search-continuation)))
 
 (defun notmuch-hello-add-saved-search (widget)
   (interactive)
@@ -200,7 +212,7 @@ diagonal."
 (defun notmuch-hello-widget-search (widget &rest ignore)
   (notmuch-search (widget-get widget
  :notmuch-search-terms)
- notmuch-search-oldest-first
+ notmuch-search-oldest-first notmuch-search-limit
  nil nil #'notmuch-hello-search-continuation))
 
 (defun notmuch-saved-search-count (search)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index c1827cc..a644cef 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -196,6 +196,7 @@ For a mouse binding, return nil."
 
 (defvar notmuch-search-mode-map
   (let ((map (make-sparse-keymap)))
+(set-keymap-parent map widget-keymap)
 (define-key map "?" 'notmuch-help)
 (define-key map "q" 'notmuch-search-quit)
 (define-key map "x" 'notmuch-search-quit)
@@ -241,6 +242,7 @@ For a mouse binding, return nil."
 (defvar notmuch-search-target-thread)
 (defvar notmuch-search-target-line)
 (defvar notmuch-search-continuation)
+(defvar notmuch-search-limit)
 
 (defvar notmuch-search-disjunctive-regexp  "\\<[oO][rR]\\>")
 
@@ -375,6 +377,7 @@ Complete list of currently available key bindings:
   (make-local-variable 'notmuch-search-oldest-first)
   (make-local-variable 'notmuch-search-target-thread)
   (make-local-variable 'notmuch-search-target-line)
+  (make-local-variable 'notmuch-search-limit)
   (set (make-local-variable 'notmuch-search-continuation) nil)
   (set (make-local-variable 'scroll-preserve-screen-position) t)
   (add-to-invisibility-spec (cons 'ellipsis t))
@@ -631,6 +634,11 @@ This function advances the next thread when finished."
(insert "End of search results.")
(if (not (= exit-status 0))
(insert (format " (process returned %d)" 
exit-status)))
+   (if (and notmuch-search-limit
+(< 0 notmuch-search-limit)
+(< notmuch-search-limit
+   (count-lines (point-min) (point-max
+   (notmuch-search-setup-buttons))
(insert "\n")
(if (and atbob
 (not (string= notmuch-sea

[PATCH 2/3] build system: add target update-versions to propagate version

2011-11-18 Thread David Bremner
From: David Bremner 

The version from file "version" is propagated to the man page and the
python bindings via sed. Note that the git version is ignored because
of the check for MAKECMDGOALS.
---
 Makefile.local |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index 10e6668..775f393 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -12,8 +12,14 @@ PACKAGE=notmuch
 
 IS_GIT=$(shell if [ -d .git ] ; then echo yes ; else echo no; fi)
 
+ifeq ($(IS_GIT),yes)
+DATE:=$(shell git log --date=short -1 --pretty=format:%cd)
+else
+DATE:=$(shell date +%F)
+endif
+
 VERSION:=$(shell cat ${srcdir}/version)
-ifeq ($filter release release-message pre-release,$(MAKECMDGOALS),)
+ifeq ($(filter release release-message pre-release 
update-versions,$(MAKECMDGOALS)),)
 ifeq ($(IS_GIT),yes)
 VERSION:=$(shell git describe --match '[0-9.]*'|sed -e s/_/~/ -e s/-/+/ -e 
s/-/~/)
 endif
@@ -87,6 +93,12 @@ $(GPG_FILE): $(SHA1_FILE)
 .PHONY: dist
 dist: $(TAR_FILE)
 
+.PHONY: update-versions
+
+update-versions:
+   sed -i "s/^.TH NOTMUCH 1.*$$/.TH NOTMUCH 1 ${DATE} \"Notmuch 
${VERSION}\"/" notmuch.1
+   sed -i "s/^__VERSION__[[:blank:]]*=.*$$/__VERSION__ = \'${VERSION}\'/" 
$(PV_FILE)
+
 # We invoke make recursively only to force ordering of our phony
 # targets in the case of parallel invocation of make (-j).
 #
-- 
1.7.7.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/3] build system: use $(filter ...) to test MAKECMDGOALS

2011-11-18 Thread David Bremner
From: David Bremner 

This makes the test easier to extend to more targets. It also corrects
a bug where "special" targets were only detected when given alone.
---
 Makefile.local |6 +-
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/Makefile.local b/Makefile.local
index 8b42136..10e6668 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -13,15 +13,11 @@ PACKAGE=notmuch
 IS_GIT=$(shell if [ -d .git ] ; then echo yes ; else echo no; fi)
 
 VERSION:=$(shell cat ${srcdir}/version)
-ifneq ($(MAKECMDGOALS),release)
-ifneq ($(MAKECMDGOALS),release-message)
-ifneq ($(MAKECMDGOALS),pre-release)
+ifeq ($filter release release-message pre-release,$(MAKECMDGOALS),)
 ifeq ($(IS_GIT),yes)
 VERSION:=$(shell git describe --match '[0-9.]*'|sed -e s/_/~/ -e s/-/+/ -e 
s/-/~/)
 endif
 endif
-endif
-endif
 
 UPSTREAM_TAG=$(subst ~,_,$(VERSION))
 DEB_TAG=debian/$(UPSTREAM_TAG)-1
-- 
1.7.7.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] test: attempt to send QUIT to smtp-dummy in case mail send failed

2011-11-18 Thread Austin Clements
LGTM.

Quoth Tomi Ollila on Nov 16 at 11:25 pm:
> If mail sending from emacs fails before it has chance to connect
> to the smtp-dummy mail server, the opportunistic QUIT message
> sending makes smtp-dummy to exit.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch