[PATCH] cli: add a tool for starting new message in the emacs ui

2014-07-04 Thread Tomi Ollila
On Tue, Jul 01 2014, David Bremner  wrote:

> From: Jani Nikula 
>
> Add a tool to start composing an email in the Notmuch Emacs UI with
> the specified subject, recipients, and message body.
> ---
>
> I added the necessary lines to conf.py to get the man pages built and
> installed
>
> I'd be happier with this if it could start emacs. I tried adding "-a
> ''" to the emacsclient invokation, but that doesn't quite work; the
> message-mode buffer is created in emacs but no frame is displayed. It
> could be a peculiarity of my emacs setup, of course.

Ah, this -a '' is new acquaintance to me -- I tried -a emacs and that
obviously did not work. It would have been nicer is there is option
to just run emacs instead... (that's what I did in that mailto: patch).

But there are plenty of other options, which needs at least be discussed,
is some tolerable subset can be agreed.

First, this will "fail"

$ emacs --daemon
$ emacsclient --eval '(progn (require 'notmuch) (notmuch-hello))'

The window is "nowhere"

$ emacsclient -nw can be used to "attach" to the emacs session and then one
can switch to the ``notmuch-hello`` -window

-- but, someone may use such a supported setup using emacs/emacsclient

This "problem" could be tacled so that if emasclient is to be used,
option (to be added) ``-nw`` is not given and stdout (or stderr) is 
a tty (test -t 1 / test -t 2), after running emacsclient print
a message to the output with content something like:

"connected to running emacs ... if the access to that emacs is hidden
 you can run ``emacsclient -nw`` to find it..."

Ok. Using emacsclient could be opportunistic -- in case using it fails
emacs(1) were used instead. The code checking this could be (*):

  unset ALTERNATE_EDITOR
  if "${EMACSCLIENT:=emacsclient}" --eval t >/dev/null 2>&1
  then  exec >/dev/null # (ok, use stderr for msg, or change this :D)
editor=$EMACSCLIENT
  else  editor=${EMACS:-emacs}
  fi

(*) to save my time, copied from 
id:1404237992-9456-1-git-send-email-tomi.ollila at iki.fi

When running emacs there is question whether to run it in background or
foreground. Probably the only always working option is to run in foreground
(unless adding an option) -- backgrounding would be possible only when all
of these apply:
   1) user did not give ``-nw`` option
   2) DISPLAY is not null or unset
   3) emacs(1) does have X support!

3 cannot be determined trivially.

If backgrounding were supported the only way I see it can be done is:

bg () {
   perl -e 'use POSIX; exit if fork; POSIX::setsid(); exec @ARGV' "$@"
}

Ok, then about the tool:

>  doc/conf.py|   4 ++
>  doc/man1/notmuch-emacs-mua.rst |  50 +
>  notmuch-emacs-mua  | 122 
> +
>  3 files changed, 176 insertions(+)
>  create mode 100644 doc/man1/notmuch-emacs-mua.rst
>  create mode 100755 notmuch-emacs-mua
>
> diff --git a/doc/conf.py b/doc/conf.py
> index 70ba1b8..8ee19f4 100644
> --- a/doc/conf.py
> +++ b/doc/conf.py
> @@ -74,6 +74,10 @@ man_pages = [
>  u'creates a plain-text dump of the tags of each message',
>  [u'Carl Worth and many others'], 1),
>  
> +('man1/notmuch-emacs-mua','notmuch-emacs-mua',
> +u'send mail with notmuch and emacs',
> +[u'Carl Worth and many others'], 1),
> +
>  ('man5/notmuch-hooks','notmuch-hooks',
>  u'hooks for notmuch',
>  [u'Carl Worth and many others'], 5),
> diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
> new file mode 100644
> index 000..6e63818
> --- /dev/null
> +++ b/doc/man1/notmuch-emacs-mua.rst
> @@ -0,0 +1,50 @@
> +=
> +notmuch-emacs-mua
> +=
> +
> +SYNOPSIS
> +
> +
> +**notmuch-emacs-mua** [options ...] [ ...]
> +
> +DESCRIPTION
> +===
> +
> +Start composing an email in the Notmuch Emacs UI with the specified
> +subject, recipients, and message body.
> +
> +For **notmuch-emacs-mua** to work, you need **emacsclient** and an
> +already running Emacs with a server.
> +
> +Supported options for **notmuch-emacs-mua** include
> +
> +``-h, --help``
> +Display help.
> +
> +``-s, --subject=``\ 
> +Specify the subject of the message.
> +
> +``--to=``\ 
> +Specify a recipient (To).
> +
> +``-c, --cc=``\ 
> +Specify a carbon-copy (Cc) recipient.
> +
> +``-b, --bcc=``\ 
> +Specify a blind-carbon-copy (Bcc) recipient.
> +
> +``-i, --body=``\ 
> +Specify a file to include into the body of the message.

-i option is consistent with mutt(1). mutt(1) does not have --body option.
therefore I'd suggest that --body takes the body content from command line
instead from file. If there is to be long-option along with -i it could
be --include or --insert..

> +
> +``--print``
> +Output the resulting elisp to stdout instead of evaluating it.
> +
> +The supported positional parameters and short options are a compatible
> +su

[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Tomi Ollila
On Fri, Jul 04 2014, Sebastian Lipp  wrote:

> Tomi Ollila  writes:
>> On Tue, Apr 09 2013, David Bremner wrote:
>>> There seems to be a few warnings:
>>>
>>> In notmuch-bbdb/snarf-from:
>>> notmuch-address.el:116:26:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In notmuch-bbdb/snarf-to:
>>> notmuch-address.el:122:29:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In end of data:
>>> notmuch-address.el:143:1:Warning: the following functions are not known to 
>>> be
>>> defined: bbdb-get-addresses, bbdb-update-records, 
>>> notmuch-show-get-header
>>>
>>> Do we need a few defvars?
>>
>> For the above set, something like:
>>
>> (defvar bbdb-get-addresses-headers)
>>
>> (declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>> props))
>>
>> (declare-function bbdb-get-addresses "bbdb-com" 
>>   (only-first-address
>>uninteresting-senders
>>get-header-content-function
>>&rest get-header-content-function-args))
>>
>> (declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>> offer-to-create))
>
> As I'd like to see this in notmuch I made the change. The patch is
> attached. As it is my first contribution to notmuch at all: Just tell me
> if I'm supposed to do it in any other way.
>
> LG
> basti

I saved this patch with 'w' in emacs mua -- and have to remove '>' From the
first line so that 'git am' could recognize it. 

There was one whitespace error -- I reapplied with

git am --whitespace=fix 
~/0001-emacs-functions-to-import-sender-or-recipient-into-B.patch

more inline (in addition to Karl's good comments)

IMO all of that can be in one patch...

>>From 522e4294258e6392a02c923b4b7e78a898986fca Mon Sep 17 00:00:00 2001
> From: Daniel Bergey 
> Date: Mon, 8 Apr 2013 19:55:04 -0500
> Subject: [PATCH] emacs: functions to import sender or recipient into BBDB [v3]
>
>>From a show buffer, notmuch-bbdb/snarf-from imports the sender into

Interestingly the line above was lost when I applied the patch -- 
probably '>From' somehow borke git am... -- should git am be more capable
there (or would the required heuristics pass the tolerance threshold) ?

> bbdb.  notmuch-bbdb/snarf-to imports all recipients.  Newly imported
> contacts are reported in the minibuffer / Messages buffer.
>
> Both functions use the BBDB parser to recognize email address formats.
>
> [v3] Fixes a few warnings as suggested by Tomi Ollila in
>  id:87vc7vgbym.fsf at zancas.localnet
> [v2] Fixes missing close parenthesis in original.
>  Spotted by Karl Fogel .
> ---
>  emacs/notmuch-address.el | 53 
> 
>  1 file changed, 53 insertions(+)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index fa65cd5..ee7b169 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -113,6 +113,59 @@ to know how address selection is made by default."
>  (when (notmuch-address-locate-command notmuch-address-command)
>(notmuch-address-message-insinuate))
>  
> +;; functions to add sender / recipients to BBDB
> +
> +(defvar bbdb-get-addresses-headers)

It would be nice to have comment where bbdb-get-addresses-headers is defined.

> +
> +(declare-function notmuch-show-get-header "notmuch-show" (header &optional 
> props))
> +
> +(declare-function bbdb-get-addresses "bbdb-com" 
> +  (only-first-address
> +   uninteresting-senders
> +   get-header-content-function
> +   &rest get-header-content-function-args))
> +
> +(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
> offer-to-create))
> +
> +(defun notmuch-bbdb/snarf-headers (headers)
> +  ;; Helper function to avoid code duplication in the two below
> +  ;; headers should have the same format as bbdb-get-addresses-headers
> +
> +  ;; bbdb-get-addresses reads these
> +  ;; Ugh, pass-by-global

Somehow this comment section looks a bit odd -- is it the empty line there,
or just that (at least part of) it is not docstring.

> +  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
> + (bbdb-get-addresses-headers headers) ; headers to read
> + (bbdb-gag-messages t)) ; suppress m/n processed message)
> +(bbdb-update-records addrs t t)))
> +
> +(defun notmuch-bbdb/snarf-from ()
> +  "Import the sender of the current message into BBDB"
> +  (interactive)
> +  (notmuch-bbdb/snarf-headers
> +   (list (assoc 'authors bbdb-get-addresses-headers
> +
> +(defun notmuch-bbdb/snarf-to ()
> +  "Import all recipients of the current message into BBDB"
> +  (interactive)
> +  (notmuch-bbdb/snarf-headers
> +   (list (assoc 'recipients bbdb-get-addresses-headers
> +
> +(defvar notmuch-bbdb/header-by-name
> +  ;; both are case sensitive
> +  '( ("From" . :From)
> + ("To" . :To)
> + ("CC" . :Cc)
> + ("BCC" . :Bcc)
> + ("Resent-From" . nil)
> + ("Reply-To" . nil)
> + ("Resent-To" . nil)
> + ("Resent-CC" . nil))
> +  "Alist for dispatching 

[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Sebastian Lipp
Karl Fogel  writes:
> Sebastian Lipp  writes:
>>As I'd like to see this in notmuch I made the change. The patch is
>>attached. As it is my first contribution to notmuch at all: Just tell me
>>if I'm supposed to do it in any other way.
>
> I think your patch includes much more than just the above, though.

True. See below on that.

>>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
>>index fa65cd5..ee7b169 100644
>>--- a/emacs/notmuch-address.el
>>+++ b/emacs/notmuch-address.el
>>@@ -113,6 +113,59 @@ to know how address selection is made by default."
>> (when (notmuch-address-locate-command notmuch-address-command)
>>   (notmuch-address-message-insinuate))
>> 
>>+;; functions to add sender / recipients to BBDB
>>+
>>+(defvar bbdb-get-addresses-headers)
>
> I think it's good to include an initial value (even an invalid
> placeholder one, if the real initialization has not happened yet), and a
> doc string.  C-h f defvar RET will say more about how to do that.

I will read into this and get back to it.

>>+(declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>>props))
>>+
>>+(declare-function bbdb-get-addresses "bbdb-com" 
>>+  (only-first-address
>>+   uninteresting-senders
>>+   get-header-content-function
>>+   &rest get-header-content-function-args))
>>+
>>+(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>>offer-to-create))
>
> At this point, your patch has accomplished what Tomi originally
> suggested.  But then the patch continues with what looks like
> substantive new code:
>
>>+(defun notmuch-bbdb/snarf-headers (headers)
>>+  ;; Helper function to avoid code duplication in the two below
>>+  ;; headers should have the same format as bbdb-get-addresses-headers
>>+
>>+  ;; bbdb-get-addresses reads these
>>+  ;; Ugh, pass-by-global
>>+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
>>+ (bbdb-get-addresses-headers headers) ; headers to read
>>+ (bbdb-gag-messages t)) ; suppress m/n processed message)
>>+(bbdb-update-records addrs t t)))
>>+
>>+(defun notmuch-bbdb/snarf-from ()
>>+  "Import the sender of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'authors bbdb-get-addresses-headers
>>+
>>+(defun notmuch-bbdb/snarf-to ()
>>+  "Import all recipients of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'recipients bbdb-get-addresses-headers
>>+
>>+(defvar notmuch-bbdb/header-by-name
>>+  ;; both are case sensitive
>>+  '( ("From" . :From)
>>+ ("To" . :To)
>>+ ("CC" . :Cc)
>>+ ("BCC" . :Bcc)
>>+ ("Resent-From" . nil)
>>+ ("Reply-To" . nil)
>>+ ("Resent-To" . nil)
>>+ ("Resent-CC" . nil))
>>+  "Alist for dispatching header symbols as used by notmuch-show-get-header
>>+from strings as used by bbdb-get-addresses")
>>+
>>+(defun notmuch-bbdb/get-header-content (name)
>>+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name
>
> It looks like new code, but I suspect what actually happened is that
> this is just the original code, somehow mis-expressed as "+" lines in
> the patch.  Is that what happened?

No, it happened by intention because what is "original" to you is not
part of my freshly cloned notmuch. So I thought good practise is to
construct *one* patch that brings the already fixed feature to the
notmuch codebase to keep it clean.

If you like better, my next patch will only base Tomis and your
suggestions on top of the "original" patch.

basti


Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Tomi Ollila
On Fri, Jul 04 2014, Sebastian Lipp  wrote:

> Tomi Ollila  writes:
>> On Tue, Apr 09 2013, David Bremner wrote:
>>> There seems to be a few warnings:
>>>
>>> In notmuch-bbdb/snarf-from:
>>> notmuch-address.el:116:26:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In notmuch-bbdb/snarf-to:
>>> notmuch-address.el:122:29:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In end of data:
>>> notmuch-address.el:143:1:Warning: the following functions are not known to 
>>> be
>>> defined: bbdb-get-addresses, bbdb-update-records, 
>>> notmuch-show-get-header
>>>
>>> Do we need a few defvars?
>>
>> For the above set, something like:
>>
>> (defvar bbdb-get-addresses-headers)
>>
>> (declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>> props))
>>
>> (declare-function bbdb-get-addresses "bbdb-com" 
>>   (only-first-address
>>uninteresting-senders
>>get-header-content-function
>>&rest get-header-content-function-args))
>>
>> (declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>> offer-to-create))
>
> As I'd like to see this in notmuch I made the change. The patch is
> attached. As it is my first contribution to notmuch at all: Just tell me
> if I'm supposed to do it in any other way.
>
> LG
> basti

I saved this patch with 'w' in emacs mua -- and have to remove '>' From the
first line so that 'git am' could recognize it. 

There was one whitespace error -- I reapplied with

git am --whitespace=fix 
~/0001-emacs-functions-to-import-sender-or-recipient-into-B.patch

more inline (in addition to Karl's good comments)

IMO all of that can be in one patch...

>>From 522e4294258e6392a02c923b4b7e78a898986fca Mon Sep 17 00:00:00 2001
> From: Daniel Bergey 
> Date: Mon, 8 Apr 2013 19:55:04 -0500
> Subject: [PATCH] emacs: functions to import sender or recipient into BBDB [v3]
>
>>From a show buffer, notmuch-bbdb/snarf-from imports the sender into

Interestingly the line above was lost when I applied the patch -- 
probably '>From' somehow borke git am... -- should git am be more capable
there (or would the required heuristics pass the tolerance threshold) ?

> bbdb.  notmuch-bbdb/snarf-to imports all recipients.  Newly imported
> contacts are reported in the minibuffer / Messages buffer.
>
> Both functions use the BBDB parser to recognize email address formats.
>
> [v3] Fixes a few warnings as suggested by Tomi Ollila in
>  id:87vc7vgbym.fsf@zancas.localnet
> [v2] Fixes missing close parenthesis in original.
>  Spotted by Karl Fogel .
> ---
>  emacs/notmuch-address.el | 53 
> 
>  1 file changed, 53 insertions(+)
>
> diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
> index fa65cd5..ee7b169 100644
> --- a/emacs/notmuch-address.el
> +++ b/emacs/notmuch-address.el
> @@ -113,6 +113,59 @@ to know how address selection is made by default."
>  (when (notmuch-address-locate-command notmuch-address-command)
>(notmuch-address-message-insinuate))
>  
> +;; functions to add sender / recipients to BBDB
> +
> +(defvar bbdb-get-addresses-headers)

It would be nice to have comment where bbdb-get-addresses-headers is defined.

> +
> +(declare-function notmuch-show-get-header "notmuch-show" (header &optional 
> props))
> +
> +(declare-function bbdb-get-addresses "bbdb-com" 
> +  (only-first-address
> +   uninteresting-senders
> +   get-header-content-function
> +   &rest get-header-content-function-args))
> +
> +(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
> offer-to-create))
> +
> +(defun notmuch-bbdb/snarf-headers (headers)
> +  ;; Helper function to avoid code duplication in the two below
> +  ;; headers should have the same format as bbdb-get-addresses-headers
> +
> +  ;; bbdb-get-addresses reads these
> +  ;; Ugh, pass-by-global

Somehow this comment section looks a bit odd -- is it the empty line there,
or just that (at least part of) it is not docstring.

> +  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
> + (bbdb-get-addresses-headers headers) ; headers to read
> + (bbdb-gag-messages t)) ; suppress m/n processed message)
> +(bbdb-update-records addrs t t)))
> +
> +(defun notmuch-bbdb/snarf-from ()
> +  "Import the sender of the current message into BBDB"
> +  (interactive)
> +  (notmuch-bbdb/snarf-headers
> +   (list (assoc 'authors bbdb-get-addresses-headers
> +
> +(defun notmuch-bbdb/snarf-to ()
> +  "Import all recipients of the current message into BBDB"
> +  (interactive)
> +  (notmuch-bbdb/snarf-headers
> +   (list (assoc 'recipients bbdb-get-addresses-headers
> +
> +(defvar notmuch-bbdb/header-by-name
> +  ;; both are case sensitive
> +  '( ("From" . :From)
> + ("To" . :To)
> + ("CC" . :Cc)
> + ("BCC" . :Bcc)
> + ("Resent-From" . nil)
> + ("Reply-To" . nil)
> + ("Resent-To" . nil)
> + ("Resent-CC" . nil))
> +  "Alist for dispatching hea

[PATCH v2] configure: use cc/c++ instead of gcc/g++

2014-07-04 Thread David Bremner
Fraser Tweedale  writes:

> Some systems (e.g. FreeBSD 10) do not ship with the GNU Compiler
> Collection.  Use generic cc/c++ instead of gcc/g++ (unless the
> CC/CXX environment variables are used).

pushed to master.

d


[PATCH 1/2] doc: build and install doxygen api docs

2014-07-04 Thread David Bremner
David Bremner  writes:

> Add to the build-man and install-man targets. We also stop hardcoding
> the version information into doxygen.cfg

There are (at least) two outstanding issues with this patch:

- configure needs to check for doxygen and disable api docs 
  if not found.

- it doesn't work for out of tree build.

So more work for me I guess.

d


Re: [PATCH v2] configure: use cc/c++ instead of gcc/g++

2014-07-04 Thread David Bremner
Fraser Tweedale  writes:

> Some systems (e.g. FreeBSD 10) do not ship with the GNU Compiler
> Collection.  Use generic cc/c++ instead of gcc/g++ (unless the
> CC/CXX environment variables are used).

pushed to master.

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


Re: [PATCH 1/2] doc: build and install doxygen api docs

2014-07-04 Thread David Bremner
David Bremner  writes:

> Add to the build-man and install-man targets. We also stop hardcoding
> the version information into doxygen.cfg

There are (at least) two outstanding issues with this patch:

- configure needs to check for doxygen and disable api docs 
  if not found.

- it doesn't work for out of tree build.

So more work for me I guess.

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


[PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Sebastian Lipp
Tomi Ollila  writes:
> On Tue, Apr 09 2013, David Bremner wrote:
>> There seems to be a few warnings:
>>
>> In notmuch-bbdb/snarf-from:
>> notmuch-address.el:116:26:Warning: reference to free variable
>> `bbdb-get-addresses-headers'
>>
>> In notmuch-bbdb/snarf-to:
>> notmuch-address.el:122:29:Warning: reference to free variable
>> `bbdb-get-addresses-headers'
>>
>> In end of data:
>> notmuch-address.el:143:1:Warning: the following functions are not known to be
>> defined: bbdb-get-addresses, bbdb-update-records, notmuch-show-get-header
>>
>> Do we need a few defvars?
>
> For the above set, something like:
>
> (defvar bbdb-get-addresses-headers)
>
> (declare-function notmuch-show-get-header "notmuch-show" (header &optional 
> props))
>
> (declare-function bbdb-get-addresses "bbdb-com" 
>   (only-first-address
>uninteresting-senders
>get-header-content-function
>&rest get-header-content-function-args))
>
> (declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
> offer-to-create))

As I'd like to see this in notmuch I made the change. The patch is
attached. As it is my first contribution to notmuch at all: Just tell me
if I'm supposed to do it in any other way.

LG
basti

-- next part --
A non-text attachment was scrubbed...
Name: 0001-emacs-functions-to-import-sender-or-recipient-into-B.patch
Type: text/x-diff
Size: 3030 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20140704/32791fd1/attachment.patch>


Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Sebastian Lipp
Karl Fogel  writes:
> Sebastian Lipp  writes:
>>As I'd like to see this in notmuch I made the change. The patch is
>>attached. As it is my first contribution to notmuch at all: Just tell me
>>if I'm supposed to do it in any other way.
>
> I think your patch includes much more than just the above, though.

True. See below on that.

>>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
>>index fa65cd5..ee7b169 100644
>>--- a/emacs/notmuch-address.el
>>+++ b/emacs/notmuch-address.el
>>@@ -113,6 +113,59 @@ to know how address selection is made by default."
>> (when (notmuch-address-locate-command notmuch-address-command)
>>   (notmuch-address-message-insinuate))
>> 
>>+;; functions to add sender / recipients to BBDB
>>+
>>+(defvar bbdb-get-addresses-headers)
>
> I think it's good to include an initial value (even an invalid
> placeholder one, if the real initialization has not happened yet), and a
> doc string.  C-h f defvar RET will say more about how to do that.

I will read into this and get back to it.

>>+(declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>>props))
>>+
>>+(declare-function bbdb-get-addresses "bbdb-com" 
>>+  (only-first-address
>>+   uninteresting-senders
>>+   get-header-content-function
>>+   &rest get-header-content-function-args))
>>+
>>+(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>>offer-to-create))
>
> At this point, your patch has accomplished what Tomi originally
> suggested.  But then the patch continues with what looks like
> substantive new code:
>
>>+(defun notmuch-bbdb/snarf-headers (headers)
>>+  ;; Helper function to avoid code duplication in the two below
>>+  ;; headers should have the same format as bbdb-get-addresses-headers
>>+
>>+  ;; bbdb-get-addresses reads these
>>+  ;; Ugh, pass-by-global
>>+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
>>+ (bbdb-get-addresses-headers headers) ; headers to read
>>+ (bbdb-gag-messages t)) ; suppress m/n processed message)
>>+(bbdb-update-records addrs t t)))
>>+
>>+(defun notmuch-bbdb/snarf-from ()
>>+  "Import the sender of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'authors bbdb-get-addresses-headers
>>+
>>+(defun notmuch-bbdb/snarf-to ()
>>+  "Import all recipients of the current message into BBDB"
>>+  (interactive)
>>+  (notmuch-bbdb/snarf-headers
>>+   (list (assoc 'recipients bbdb-get-addresses-headers
>>+
>>+(defvar notmuch-bbdb/header-by-name
>>+  ;; both are case sensitive
>>+  '( ("From" . :From)
>>+ ("To" . :To)
>>+ ("CC" . :Cc)
>>+ ("BCC" . :Bcc)
>>+ ("Resent-From" . nil)
>>+ ("Reply-To" . nil)
>>+ ("Resent-To" . nil)
>>+ ("Resent-CC" . nil))
>>+  "Alist for dispatching header symbols as used by notmuch-show-get-header
>>+from strings as used by bbdb-get-addresses")
>>+
>>+(defun notmuch-bbdb/get-header-content (name)
>>+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name
>
> It looks like new code, but I suspect what actually happened is that
> this is just the original code, somehow mis-expressed as "+" lines in
> the patch.  Is that what happened?

No, it happened by intention because what is "original" to you is not
part of my freshly cloned notmuch. So I thought good practise is to
construct *one* patch that brings the already fixed feature to the
notmuch codebase to keep it clean.

If you like better, my next patch will only base Tomis and your
suggestions on top of the "original" patch.

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


Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el.

2014-07-04 Thread Karl Fogel
Sebastian Lipp  writes:
>Tomi Ollila  writes:
>> On Tue, Apr 09 2013, David Bremner wrote:
>>> There seems to be a few warnings:
>>>
>>> In notmuch-bbdb/snarf-from:
>>> notmuch-address.el:116:26:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In notmuch-bbdb/snarf-to:
>>> notmuch-address.el:122:29:Warning: reference to free variable
>>> `bbdb-get-addresses-headers'
>>>
>>> In end of data:
>>> notmuch-address.el:143:1:Warning: the following functions are not known to 
>>> be
>>> defined: bbdb-get-addresses, bbdb-update-records, 
>>> notmuch-show-get-header
>>>
>>> Do we need a few defvars?
>>
>> For the above set, something like:
>>
>> (defvar bbdb-get-addresses-headers)
>>
>> (declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>> props))
>>
>> (declare-function bbdb-get-addresses "bbdb-com" 
>>   (only-first-address
>>uninteresting-senders
>>get-header-content-function
>>&rest get-header-content-function-args))
>>
>> (declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>> offer-to-create))
>
>As I'd like to see this in notmuch I made the change. The patch is
>attached. As it is my first contribution to notmuch at all: Just tell me
>if I'm supposed to do it in any other way.

I think your patch includes much more than just the above, though.

(It helps to include a log message with the patch -- then people can see
what you intended the code change to be, and compare that against the
code change the patch actually makes.)

>diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
>index fa65cd5..ee7b169 100644
>--- a/emacs/notmuch-address.el
>+++ b/emacs/notmuch-address.el
>@@ -113,6 +113,59 @@ to know how address selection is made by default."
> (when (notmuch-address-locate-command notmuch-address-command)
>   (notmuch-address-message-insinuate))
> 
>+;; functions to add sender / recipients to BBDB
>+
>+(defvar bbdb-get-addresses-headers)

I think it's good to include an initial value (even an invalid
placeholder one, if the real initialization has not happened yet), and a
doc string.  C-h f defvar RET will say more about how to do that.

(By the way, this isn't a user-customizeable variable, right?  If it
were, then `defcustom' would be better than `defvar'.)

>+(declare-function notmuch-show-get-header "notmuch-show" (header &optional 
>props))
>+
>+(declare-function bbdb-get-addresses "bbdb-com" 
>+  (only-first-address
>+   uninteresting-senders
>+   get-header-content-function
>+   &rest get-header-content-function-args))
>+
>+(declare-function bbdb-update-records "bbdb-com" (addrs auto-create-p 
>offer-to-create))

At this point, your patch has accomplished what Tomi originally
suggested.  But then the patch continues with what looks like
substantive new code:

>+(defun notmuch-bbdb/snarf-headers (headers)
>+  ;; Helper function to avoid code duplication in the two below
>+  ;; headers should have the same format as bbdb-get-addresses-headers
>+
>+  ;; bbdb-get-addresses reads these
>+  ;; Ugh, pass-by-global
>+  (let ((addrs (bbdb-get-addresses nil nil 'notmuch-bbdb/get-header-content))
>+  (bbdb-get-addresses-headers headers) ; headers to read
>+  (bbdb-gag-messages t)) ; suppress m/n processed message)
>+(bbdb-update-records addrs t t)))
>+
>+(defun notmuch-bbdb/snarf-from ()
>+  "Import the sender of the current message into BBDB"
>+  (interactive)
>+  (notmuch-bbdb/snarf-headers
>+   (list (assoc 'authors bbdb-get-addresses-headers
>+
>+(defun notmuch-bbdb/snarf-to ()
>+  "Import all recipients of the current message into BBDB"
>+  (interactive)
>+  (notmuch-bbdb/snarf-headers
>+   (list (assoc 'recipients bbdb-get-addresses-headers
>+
>+(defvar notmuch-bbdb/header-by-name
>+  ;; both are case sensitive
>+  '( ("From" . :From)
>+ ("To" . :To)
>+ ("CC" . :Cc)
>+ ("BCC" . :Bcc)
>+ ("Resent-From" . nil)
>+ ("Reply-To" . nil)
>+ ("Resent-To" . nil)
>+ ("Resent-CC" . nil))
>+  "Alist for dispatching header symbols as used by notmuch-show-get-header
>+from strings as used by bbdb-get-addresses")
>+
>+(defun notmuch-bbdb/get-header-content (name)
>+  (notmuch-show-get-header (cdr (assoc name notmuch-bbdb/header-by-name

It looks like new code, but I suspect what actually happened is that
this is just the original code, somehow mis-expressed as "+" lines in
the patch.  Is that what happened?

Best,
-Karl
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch