72/f60a237cbac9e3ff0962e05ac52c03fc5ec71e

   1 Return-Path: <amdragon@mit.edu>\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 8D21C429E27\r
   6         for <notmuch@notmuchmail.org>; Sun,  6 Jan 2013 13:12:43 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id E-JGUrVOur4V for <notmuch@notmuchmail.org>;\r
  16         Sun,  6 Jan 2013 13:12:34 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-1.mit.edu (DMZ-MAILSEC-SCANNER-1.MIT.EDU\r
  18         [18.9.25.12])\r
  19         by olra.theworths.org (Postfix) with ESMTP id ABDB1431FD4\r
  20         for <notmuch@notmuchmail.org>; Sun,  6 Jan 2013 13:12:34 -0800 (PST)\r
  21 X-AuditID: 1209190c-b7fa86d000001d37-9f-50e9e8c22cac\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 38.F7.07479.2C8E9E05; Sun,  6 Jan 2013 16:12:34 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id r06LCXuO019829; \r
  27         Sun, 6 Jan 2013 16:12:34 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id r06LCV2P025193\r
  32         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  33         Sun, 6 Jan 2013 16:12:33 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1TrxWB-0002bC-I2; Sun, 06 Jan 2013 16:12:31 -0500\r
  37 From: Austin Clements <amdragon@MIT.EDU>\r
  38 To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org\r
  39 Subject: Re: [PATCH v3] emacs: Use the minibuffer for CLI error reporting\r
  40 In-Reply-To: <87ehi1j32b.fsf@qmul.ac.uk>\r
  41 References: <1357174222-25132-1-git-send-email-amdragon@mit.edu>\r
  42         <1357249669-9706-1-git-send-email-amdragon@mit.edu>\r
  43         <87ehi1j32b.fsf@qmul.ac.uk>\r
  44 User-Agent: Notmuch/0.14+236~gf64406d (http://notmuchmail.org) Emacs/23.4.1\r
  45         (i486-pc-linux-gnu)\r
  46 Date: Sun, 06 Jan 2013 16:12:31 -0500\r
  47 Message-ID: <87y5g6j9s0.fsf@awakening.csail.mit.edu>\r
  48 MIME-Version: 1.0\r
  49 Content-Type: text/plain; charset=us-ascii\r
  50 X-Brightmail-Tracker:\r
  51  H4sIAAAAAAAAA+NgFupileLIzCtJLcpLzFFi42IR4hTV1j304mWAwYPHPBY3WrsZLfbs87JY\r
  52         PZfH4vrNmcwOLB53T3N57Jx1l93j2apbzB5bDr1nDmCJ4rJJSc3JLEst0rdL4MrY/u4Ra8EG\r
  53         qYo326eyNzBOEu1i5OSQEDCRuPpuAQuELSZx4d56NhBbSGAfo8Sf+0A2F5C9nlHiUe9WFgjn\r
  54         ApPE7Ib/7BDOEkaJjtlXmUFa2AQ0JLbtX84IYosIuEo8/fYZLM4sYCixZdpbdhBbWMBToqWh\r
  55         DWwFJ1D95DdLWSDWTWCUmLlfHsQWFYiXeH7vG1icRUBVYuq5v2A2L9CpB39PY4WwBSVOznzC\r
  56         AjFfS+LGv5dMExgFZyFJzUKSWsDItIpRNiW3Sjc3MTOnODVZtzg5MS8vtUjXUC83s0QvNaV0\r
  57         EyM4mCV5djC+Oah0iFGAg1GJh/fCzhcBQqyJZcWVuYcYJTmYlER59959GSDEl5SfUpmRWJwR\r
  58         X1Sak1p8iFGCg1lJhHffMaAcb0piZVVqUT5MSpqDRUmc93LKTX8hgfTEktTs1NSC1CKYrAwH\r
  59         h5IEb/pzoEbBotT01Iq0zJwShDQTByfIcB6g4YYgNbzFBYm5xZnpEPlTjLocDS9vPGUUYsnL\r
  60         z0uVEuflegpUJABSlFGaBzcHloReMYoDvSXMawkyigeYwOAmvQJawgS0JPXxc5AlJYkIKakG\r
  61         RjfP+fnWkeLiOvab42Uu3D33wJuzwCUpXdh9ovLlk4KZF6svmH+3EXSzmu5t4Du77/zF2NDj\r
  62         3pIWZgf9eP6mW3z6nsvivcnw26Ur4v4FBcobwrynt+gf+yVQ8+Thxt2yV6yUckPORtv8fFnZ\r
  63         rl+2b9PXlAyTEtvv0+e/mHnGvPoRo83X+LgGJZbijERDLeai4kQAO9Efhx0DAAA=\r
  64 X-BeenThere: notmuch@notmuchmail.org\r
  65 X-Mailman-Version: 2.1.13\r
  66 Precedence: list\r
  67 List-Id: "Use and development of the notmuch mail system."\r
  68         <notmuch.notmuchmail.org>\r
  69 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  71 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  72 List-Post: <mailto:notmuch@notmuchmail.org>\r
  73 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  74 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  76 X-List-Received-Date: Sun, 06 Jan 2013 21:12:43 -0000\r
  77 \r
  78 On Thu, 03 Jan 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
  79 > On Thu, 03 Jan 2013, Austin Clements <amdragon@MIT.EDU> wrote:\r
  80 >> We recently switched to popping up a buffer to report CLI errors, but\r
  81 >> this was too intrusive, especially for transient errors and especially\r
  82 >> since we made fewer things ignore errors.  This patch changes this to\r
  83 >> display a basic error message in the minibuffer (using Emacs' usual\r
  84 >> error handling path) and, if there are additional details, to log\r
  85 >> these to a separate error buffer and reference the error buffer from\r
  86 >> the minibuffer message.  This is more in line with how Emacs typically\r
  87 >> handles errors, but makes the details available to the user without\r
  88 >> flooding them with the details.\r
  89 >>\r
  90 >> Given this split, we pare down the basic message and make it more\r
  91 >> user-friendly, and also make the verbose message even more detailed\r
  92 >> (and more debugging-oriented).\r
  93 >> ---\r
  94 >>\r
  95 >> This version fixes two hard-coded paths in the tests.\r
  96 >\r
  97 >\r
  98 > This version looks good to me but I have one query we may like to\r
  99 > consider. \r
 100 >\r
 101 > At the moment notmuch-call-notmuch-process returns the stderr mixed with\r
 102 > stdout and we might like to separate that out (particularly as the error\r
 103 > message lists stderr and stdout separately and in this case it all gets\r
 104 > called stdout).\r
 105 \r
 106 This sounds like a good idea to me, though I'd rather do it as a\r
 107 separate patch.  Your patch looks good to me (modulo commit message,\r
 108 obviously).  Care to roll an official patch?\r
 109 \r
 110 As I mentioned elsewhere, there are several direct calls to notmuch that\r
 111 don't go through any of this error handling (for example,\r
 112 `notmuch-call-notmuch-process' is only used in 'notmuch-tag').  It would\r
 113 be great if Someone (TM) cleaned this up.\r
 114 \r
 115 > I attach a patch that does this: I am not really familiar with this so I\r
 116 > just took Austin's code from notmuch-call-notmuch-json.\r
 117 >\r
 118 > Austin: obviously feel free to fold this into your patch if you think\r
 119 > appropriate.\r
 120 >\r
 121 > Best wishes\r
 122 >\r
 123 > Mark\r
 124 >\r
 125 > From b73395c8efb57111bd4de281797004747de6c2ed Mon Sep 17 00:00:00 2001\r
 126 > From: Mark Walters <markwalters1009@gmail.com>\r
 127 > Date: Thu, 3 Jan 2013 22:25:02 +0000\r
 128 > Subject: [PATCH] tweak notmuch-call-notmuch-process\r
 129 >\r
 130 > ---\r
 131 >  emacs/notmuch.el |   10 +++++++---\r
 132 >  1 files changed, 7 insertions(+), 3 deletions(-)\r
 133 >\r
 134 > diff --git a/emacs/notmuch.el b/emacs/notmuch.el\r
 135 > index c98a4fe..4f7ee2c 100644\r
 136 > --- a/emacs/notmuch.el\r
 137 > +++ b/emacs/notmuch.el\r
 138 > @@ -540,9 +540,13 @@ If notmuch exits with a non-zero status, output from the process\r
 139 >  will appear in a buffer named \"*Notmuch errors*\" and an error\r
 140 >  will be signaled."\r
 141 >    (with-temp-buffer\r
 142 > -    (let ((status (apply #'call-process notmuch-command nil t nil args)))\r
 143 > -      (notmuch-check-exit-status status (cons notmuch-command args)\r
 144 > -                              (buffer-string)))))\r
 145 > +    (let ((err-file (make-temp-file "nmerr")))\r
 146 > +      (unwind-protect\r
 147 > +       (let ((status (apply #'call-process\r
 148 > +                            notmuch-command nil (list t err-file) nil args)))\r
 149 > +         (notmuch-check-exit-status status (cons notmuch-command args)\r
 150 > +                                    (buffer-string) err-file))\r
 151 > +     (delete-file err-file)))))\r
 152 >  \r
 153 >  (defun notmuch-search-set-tags (tags &optional pos)\r
 154 >    (let ((new-result (plist-put (notmuch-search-get-result pos) :tags tags)))\r
 155 > -- \r
 156 > 1.7.9.1\r