From: Karl Fogel Date: Fri, 4 Jul 2014 00:56:33 +0000 (+1900) Subject: Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el. X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=e9d193a1d7cd70da9b9667e61fed97bcdf1d15c3;p=notmuch-archives.git Re: [PATCH] emacs: add missing paren to fix defun in notmuch-address.el. --- diff --git a/e0/509cab1ee23c1946a2a18d437eb6a608a35eca b/e0/509cab1ee23c1946a2a18d437eb6a608a35eca new file mode 100644 index 000000000..cbf3d1594 --- /dev/null +++ b/e0/509cab1ee23c1946a2a18d437eb6a608a35eca @@ -0,0 +1,200 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 1B3F9431FAF + for ; Thu, 3 Jul 2014 17:56:39 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.699 +X-Spam-Level: +X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5 + tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, FREEMAIL_FROM=0.001, + RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id cKn2NNlR-FBY for ; + Thu, 3 Jul 2014 17:56:35 -0700 (PDT) +Received: from mail-ig0-f172.google.com (mail-ig0-f172.google.com + [209.85.213.172]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id AFD7B431FAE + for ; Thu, 3 Jul 2014 17:56:35 -0700 (PDT) +Received: by mail-ig0-f172.google.com with SMTP id hn18so8263255igb.17 + for ; Thu, 03 Jul 2014 17:56:35 -0700 (PDT) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; + h=sender:from:to:cc:subject:references:reply-to:date:in-reply-to + :message-id:user-agent:mime-version:content-type; + bh=BKgDvd3dWuQBSuRg06t9jXorWKEHcSn9AA5UOOSML+Q=; + b=dS7SxtjGgngxWbJpe1poeH3kLgsHbcIdfEEvjlYoy7vC2WO6P7cTlRuBtiU5grsUlU + cq8A4J3MzqU2VgLkgdNowx3g1kpvN90LabZshmFliCzXpj2OWRCsZ0+tEOGk0bx1zEY0 + MsbgdpqgOkORBw/zwBWPJLBpEpH8Z+EUqo76eCIy6wOxL3huaqjcui3YrmhlR0QqGANM + wVxzFvy/U4Rc4tn1HynP+8rZiJ94uEW2J//XsDO2nK2CSQJA2jNIBEvnJVrtc1hcgkOo + /MfL2UNh0/b7aXKKCelMqcXrIkqH4mzZApoTYTwa376fkOMRycSacHe/H4SJEDXILBaq + aSLw== +X-Received: by 10.42.178.1 with SMTP id bk1mr5188239icb.75.1404435395197; + Thu, 03 Jul 2014 17:56:35 -0700 (PDT) +Received: from ktab.red-bean.com + (74-92-190-113-Illinois.hfc.comcastbusiness.net. [74.92.190.113]) + by mx.google.com with ESMTPSA id + u10sm57440285igz.21.2014.07.03.17.56.34 for + (version=TLSv1.2 cipher=RC4-SHA bits=128/128); + Thu, 03 Jul 2014 17:56:34 -0700 (PDT) +Sender: Karl Fogel +From: Karl Fogel +To: Sebastian Lipp +Subject: Re: [PATCH] emacs: add missing paren to fix defun in + notmuch-address.el. +References: <87wqsfik5l.fsf@floss.red-bean.com> + <87mwtah2hd.fsf@zancas.localnet> <877gkeau42.fsf@floss.red-bean.com> + <8738v1jtzc.fsf@nikula.org> + <87k3odgutd.fsf@zancas.localnet> <87ip3wbj7g.fsf@mcs.anl.gov> + <87vc7vgbym.fsf@zancas.localnet> + + <87pphmc604.fsf@verb.i-did-not-set--mail-host-address--so-tickle-me> +Date: Thu, 03 Jul 2014 19:56:33 -0500 +In-Reply-To: + <87pphmc604.fsf@verb.i-did-not-set--mail-host-address--so-tickle-me> + (Sebastian Lipp's message of "Fri, 04 Jul 2014 02:38:51 +0200") +Message-ID: <87oax6vt4u.fsf@ktab.red-bean.com> +User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) +MIME-Version: 1.0 +Content-Type: text/plain +X-Mailman-Approved-At: Fri, 04 Jul 2014 00:58:35 -0700 +Cc: Tomi Ollila , notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +Reply-To: Karl Fogel +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Fri, 04 Jul 2014 00:56:39 -0000 + +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