40/2f7e3091adcf461b218de36325116b37aac492

   1 Return-Path: <m.walters@qmul.ac.uk>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 4451F431FDA\r
   6         for <notmuch@notmuchmail.org>; Sun, 30 Jun 2013 14:15:17 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id naruzOdDOP8y for <notmuch@notmuchmail.org>;\r
  17         Sun, 30 Jun 2013 14:15:10 -0700 (PDT)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id F2069431FAF\r
  22         for <notmuch@notmuchmail.org>; Sun, 30 Jun 2013 14:15:09 -0700 (PDT)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1UtOxb-00011D-T4; Sun, 30 Jun 2013 22:15:06 +0100\r
  27 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost)\r
  28         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71)\r
  29         (envelope-from <m.walters@qmul.ac.uk>)\r
  30         id 1UtOxb-0002DO-J6; Sun, 30 Jun 2013 22:15:03 +0100\r
  31 From: Mark Walters <markwalters1009@gmail.com>\r
  32 To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH v2 2/3] contrib: pick: remove hack\r
  34         notmuch-pick-show-match-message-with-wait\r
  35 In-Reply-To: <87li5rtvbh.fsf@zancas.localnet>\r
  36 References: <1372582512-10777-1-git-send-email-markwalters1009@gmail.com>\r
  37         <1372582512-10777-3-git-send-email-markwalters1009@gmail.com>\r
  38         <87li5rtvbh.fsf@zancas.localnet>\r
  39 User-Agent: Notmuch/0.15.2+192~g8222af3 (http://notmuchmail.org) Emacs/23.4.1\r
  40         (x86_64-pc-linux-gnu)\r
  41 Date: Sun, 30 Jun 2013 22:15:01 +0100\r
  42 Message-ID: <87hagf5mi2.fsf@qmul.ac.uk>\r
  43 MIME-Version: 1.0\r
  44 Content-Type: text/plain; charset=us-ascii\r
  45 X-Sender-Host-Address: 93.97.24.31\r
  46 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  47 X-QM-Body-MD5: 38c225b8be9042716392fff3aa01a923 (of first 20000 bytes)\r
  48 X-SpamAssassin-Score: 0.0\r
  49 X-SpamAssassin-SpamBar: /\r
  50 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  51         determine if it is\r
  52         spam. We require at least 5.0 points to mark a message as spam.\r
  53         This message scored 0.0 points. Summary of the scoring: \r
  54         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  55         provider *      (markwalters1009[at]gmail.com)\r
  56         *  0.0 AWL AWL: From: address is in the auto white-list\r
  57 X-QM-Scan-Virus: ClamAV says the message is clean\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.13\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62         <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Sun, 30 Jun 2013 21:15:17 -0000\r
  71 \r
  72 \r
  73 Hi\r
  74 \r
  75 Many thanks for the review!\r
  76 \r
  77 On Sun, 30 Jun 2013, David Bremner <david@tethera.net> wrote:\r
  78 > Mark Walters <markwalters1009@gmail.com> writes:\r
  79 >\r
  80 >> +(defvar notmuch-pick-open-target nil)\r
  81 >> +(make-variable-buffer-local 'notmuch-pick-open-target)\r
  82 >\r
  83 > What do people think about adding a code style suggestion/requirement\r
  84 > for elisp that all variables have docstrings, even if intended for\r
  85 > internal use?  It's true the existing code doesn't really meet this\r
  86 > standard.\r
  87 \r
  88 I think this would be a good idea (but see below): I assume this is\r
  89 anything defined with a defvar?\r
  90 \r
  91 >>  (defvar notmuch-pick-buffer-name nil)\r
  92 >>  (make-variable-buffer-local 'notmuch-pick-buffer-name)\r
  93 >>  ;; This variable is the window used for the message pane. It is set\r
  94 >> @@ -349,8 +351,8 @@ Does NOT change the database."\r
  95 >>    (notmuch-pick (notmuch-search-find-thread-id)\r
  96 >>                  notmuch-search-query-string\r
  97 >>              nil\r
  98 >> -                (notmuch-prettify-subject (notmuch-search-find-subject)))\r
  99 >> -  (notmuch-pick-show-match-message-with-wait))\r
 100 >> +                (notmuch-prettify-subject (notmuch-search-find-subject))\r
 101 >> +            t))\r
 102 >\r
 103 > I think my previous complaint can be reformulated as (essentially) both\r
 104 > notmuch-pick and notmuch-pick-open-target could use (better) docstrings.\r
 105 \r
 106 I will send a patch to add a docstring to the main notmuch-pick function\r
 107 as a reply to this message. The exact style was unclear (we seem to do\r
 108 different things in different places).\r
 109 \r
 110 I am not sure what the best way to document the variable is: there are\r
 111 several variables that are essentially buffer local versions of the\r
 112 arguments passed to notmuch-pick. Should these duplicate the\r
 113 documentation? Exactly the same situation occurs with notmuch-show in\r
 114 notmuch-show.el and notmuch-search in notmuch.el and in both those cases\r
 115 the functions are well documented but the variables are not documented.\r
 116 \r
 117 Any suggestions?\r
 118 \r
 119 Best wishes\r
 120 \r
 121 Mark\r
 122 \r
 123 \r
 124 \r
 125 > As you say, the hack removed is quite horrible, so I'd be willing to\r
 126 > merge the patches anyway. OTOH, more documentation might make it so that\r
 127 > more than one person can understand the notmuch-pick code.\r
 128 >\r
 129 > d\r