78/612d8cb556204a26bba408657024d5f0299090

   1 Return-Path: <aaronecay@gmail.com>\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 910A5431FD0\r
   6         for <notmuch@notmuchmail.org>; Sun, 25 Dec 2011 20:11:33 -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.799\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] 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 xqmaRwVAwtit for <notmuch@notmuchmail.org>;\r
  17         Sun, 25 Dec 2011 20:11:32 -0800 (PST)\r
  18 Received: from mail-qw0-f46.google.com (mail-qw0-f46.google.com\r
  19         [209.85.216.46]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id B58B6431FB6\r
  22         for <notmuch@notmuchmail.org>; Sun, 25 Dec 2011 20:11:32 -0800 (PST)\r
  23 Received: by qadc12 with SMTP id c12so7147098qad.5\r
  24         for <notmuch@notmuchmail.org>; Sun, 25 Dec 2011 20:11:31 -0800 (PST)\r
  25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma;\r
  26         h=from:to:cc:subject:in-reply-to:references:user-agent:date\r
  27         :message-id:mime-version:content-type;\r
  28         bh=EHNrDNhiPDljSIlETiXFyLcrbJE47F9o7KxuJ9OeaGQ=;\r
  29         b=Ebg1fw1v/MRCp7BVa2VyZ8rm2aZjcNG9CNHNGMxyVHLIcjomo9eE3m+OzldWwF1XoG\r
  30         zXqvZ7+p6KCxZuCQUL0BHC5d9RDFXPDlZafF+Y21SyarUwlB/bPq/1FVvhUNN7PnRMHZ\r
  31         zfDPsGT4uEjIlVYVWIQr3z4RAl+dthpZe1t10=\r
  32 Received: by 10.224.186.8 with SMTP id cq8mr27794838qab.45.1324872691143;\r
  33         Sun, 25 Dec 2011 20:11:31 -0800 (PST)\r
  34 Received: from localhost (24-158-179-191.dhcp.jcsn.tn.charter.com.\r
  35         [24.158.179.191])\r
  36         by mx.google.com with ESMTPS id ft9sm41769209qab.20.2011.12.25.20.11.29\r
  37         (version=TLSv1/SSLv3 cipher=OTHER);\r
  38         Sun, 25 Dec 2011 20:11:30 -0800 (PST)\r
  39 From: Aaron Ecay <aaronecay@gmail.com>\r
  40 To: Austin Clements <amdragon@MIT.EDU>, David Edmondson <dme@dme.org>\r
  41 Subject: Re: [RFC][PATCH v4] emacs: Re-implement advance/rewind functions of\r
  42         notmuch-show-mode.\r
  43 In-Reply-To: <20111225010635.GG1927@mit.edu>\r
  44 References: <id:"1324553312-10972-1-git-send-email-dme@dme.org">\r
  45         <1324665712-2419-1-git-send-email-dme@dme.org>\r
  46         <20111225010635.GG1927@mit.edu>\r
  47 User-Agent: Notmuch/0.10.1+56~gd709fd6 (http://notmuchmail.org)\r
  48         Emacs/24.0.92.3 (i386-apple-darwin10.8.0)\r
  49 Date: Sun, 25 Dec 2011 23:11:27 -0500\r
  50 Message-ID: <m2zkegt1jk.fsf@gmail.com>\r
  51 MIME-Version: 1.0\r
  52 Content-Type: text/plain\r
  53 Cc: notmuch@notmuchmail.org\r
  54 X-BeenThere: notmuch@notmuchmail.org\r
  55 X-Mailman-Version: 2.1.13\r
  56 Precedence: list\r
  57 List-Id: "Use and development of the notmuch mail system."\r
  58         <notmuch.notmuchmail.org>\r
  59 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  60         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  61 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  62 List-Post: <mailto:notmuch@notmuchmail.org>\r
  63 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  64 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  65         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  66 X-List-Received-Date: Mon, 26 Dec 2011 04:11:33 -0000\r
  67 \r
  68 On Sat, 24 Dec 2011 20:06:35 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
  69 > Awesome.  This looks significantly cleaner.  I think this is worth\r
  70 > pushing for the comment you added to notmuch-show-advance alone.\r
  71 \r
  72 +1\r
  73 \r
  74 > Quoth David Edmondson on Dec 23 at  6:41 pm:\r
  75 > > The advance/rewind functions had become complex, which made it hard to\r
  76 > > determine how they are expected to behave. Re-implement them simply in\r
  77 > > order to poll user-experience and expectation.\r
  78 > > ---\r
  79 > > \r
  80 > > Switched back to using `previous-single-char-property-change' now that\r
  81 > > Aaron explained it. Fix a bug rewinding when the start of the current\r
  82 > > message is visible.\r
  83 > > \r
  84 > >  emacs/notmuch-show.el |  132 +++++++++++++++++++++++++++----------------------\r
  85 > >  1 files changed, 73 insertions(+), 59 deletions(-)\r
  86 > > \r
  87 > > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el\r
  88 > > index 46525aa..e914ce1 100644\r
  89 > > --- a/emacs/notmuch-show.el\r
  90 > > +++ b/emacs/notmuch-show.el\r
  91 > > @@ -1156,38 +1156,56 @@ Some useful entries are:\r
  92 > >  ;; Commands typically bound to keys.\r
  93 > >  \r
  94 > >  (defun notmuch-show-advance ()\r
  95 > > -  "Advance through thread.\r
  96 > > +  "Advance through the current thread.\r
  97 > >  \r
  98 > > -If the current message in the thread is not yet fully visible,\r
  99 > > -scroll by a near screenful to read more of the message.\r
 100 > > +Scroll the current message if the end of it is not visible,\r
 101 > > +otherwise move to the next message.\r
 102 > >  \r
 103 > > -Otherwise, (the end of the current message is already within the\r
 104 > > -current window), advance to the next open message."\r
 105 > > +Return `t' if we are at the end of the last message, otherwise\r
 106 > > +`nil'."\r
 107 > >    (interactive)\r
 108 > > -  (let* ((end-of-this-message (notmuch-show-message-bottom))\r
 109 > > -    (visible-end-of-this-message (1- end-of-this-message))\r
 110 > > -    (ret nil))\r
 111 > > -    (while (invisible-p visible-end-of-this-message)\r
 112 > > -      (setq visible-end-of-this-message\r
 113 > > -       (previous-single-char-property-change visible-end-of-this-message\r
 114 > > -                                             'invisible)))\r
 115 > > -    (cond\r
 116 > > -     ;; Ideally we would test `end-of-this-message' against the result\r
 117 > > -     ;; of `window-end', but that doesn't account for the fact that\r
 118 > > -     ;; the end of the message might be hidden.\r
 119 > > -     ((and visible-end-of-this-message\r
 120 > > -      (> visible-end-of-this-message (window-end)))\r
 121 > > -      ;; The bottom of this message is not visible - scroll.\r
 122 > > -      (scroll-up nil))\r
 123 > > -\r
 124 > > -     ((not (= end-of-this-message (point-max)))\r
 125 > > -      ;; This is not the last message - move to the next visible one.\r
 126 > > -      (notmuch-show-next-open-message))\r
 127 > > -\r
 128 > > -     (t\r
 129 > > -      ;; This is the last message - change the return value\r
 130 > > -      (setq ret t)))\r
 131 > > -    ret))\r
 132 > > +  (cond\r
 133 > > +   ((eobp)\r
 134 > > +    ;; We are at the end of the buffer - move to the next thread.\r
 135 > > +    t)\r
 136 > > +\r
 137 > > +   ;; Ideally we would simply do:\r
 138 > > +   ;; \r
 139 > \r
 140 > Tailing whitespace.\r
 141 > \r
 142 > > +   ;;      ((> (notmuch-show-message-bottom) (window-end))\r
 143 > > +   ;; \r
 144 > \r
 145 > More trailing whitespace.\r
 146 > \r
 147 > > +   ;; here, but that fails if the trailing text in the buffer is\r
 148 > > +   ;; invisible (`window-end' returns the last _visible_ character,\r
 149 > > +   ;; which can then be smaller than `notmuch-show-message-bottom').\r
 150 > > +   ;;\r
 151 > > +   ;; So we need to find the last visible character of the message. We\r
 152 > > +   ;; do this by searching backwards from\r
 153 > > +   ;; `notmuch-show-message-bottom' for changes in the `invisible'\r
 154 > > +   ;; property until we find a non-invisible character. When we find\r
 155 > > +   ;; such a character we test to see whether it is visible in the\r
 156 > > +   ;; window.\r
 157 > > +   ;;\r
 158 > > +   ;; Properties change between characters - the return value of\r
 159 > > +   ;; `previous-single-char-property-change' points to the first\r
 160 > > +   ;; character _inside_ the region with the `invisible' property\r
 161 > > +   ;; set. To allow for this we step backwards one character upon\r
 162 > > +   ;; finding the start of the invisible region.\r
 163 > > +\r
 164 > > +   ((> (let ((visible-bottom (notmuch-show-message-bottom)))\r
 165 > > +    (while (invisible-p visible-bottom)\r
 166 > > +      (setq visible-bottom (max (point-min)\r
 167 > > +                                (1- (previous-single-char-property-change\r
 168 > > +                                     visible-bottom 'invisible)))))\r
 169 > > +    visible-bottom) (window-end))\r
 170 \r
 171 Can this (let...) be lifted out of the (cond...)?  IMO it is very\r
 172 confusing to be doing non-trivial computation in the test portion of a\r
 173 cond form.\r
 174 \r
 175 > > +    ;; The end of this message is not visible - scroll to show more of\r
 176 > > +    ;; it.\r
 177 > > +    (scroll-up)\r
 178 > > +    nil)\r
 179 > > +\r
 180 > > +   (t\r
 181 > > +    ;; All of the current message has been seen - show the start of\r
 182 > > +    ;; the next open message.\r
 183 > > +    (notmuch-show-next-open-message)\r
 184 > > +    nil)))\r
 185 > >  \r
 186 > >  (defun notmuch-show-advance-and-archive ()\r
 187 > >    "Advance through thread and archive.\r
 188 > > @@ -1201,44 +1219,40 @@ from each message), kills the buffer, and displays the next\r
 189 > >  thread from the search from which this thread was originally\r
 190 > >  shown."\r
 191 > >    (interactive)\r
 192 > > -  (if (notmuch-show-advance)\r
 193 > > -      (notmuch-show-archive-thread)))\r
 194 > > +  (when (notmuch-show-advance)\r
 195 > > +    (notmuch-show-archive-thread)))\r
 196 > >  \r
 197 > >  (defun notmuch-show-rewind ()\r
 198 > > -  "Backup through the thread, (reverse scrolling compared to \\[notmuch-show-advance-and-archive]).\r
 199 > > +  "Move backwards through a thread, the counterpart to \\[notmuch-show-advance-and-archive]."\r
 200 > >  \r
 201 > > -Specifically, if the beginning of the previous email is fewer\r
 202 > > -than `window-height' lines from the current point, move to it\r
 203 > > -just like `notmuch-show-previous-message'.\r
 204 > > -\r
 205 > > -Otherwise, just scroll down a screenful of the current message.\r
 206 > > -\r
 207 > > -This command does not modify any message tags, (it does not undo\r
 208 > > -any effects from previous calls to\r
 209 > > -`notmuch-show-advance-and-archive'."\r
 210 > >    (interactive)\r
 211 > > -  (let ((start-of-message (notmuch-show-message-top))\r
 212 > > -   (start-of-window (window-start)))\r
 213 > > +  (let ((start-of-message (notmuch-show-message-top)))\r
 214 > >      (cond\r
 215 > > -      ;; Either this message is properly aligned with the start of the\r
 216 > > -      ;; window or the start of this message is not visible on the\r
 217 > > -      ;; screen - scroll.\r
 218 > > -     ((or (= start-of-message start-of-window)\r
 219 > > -     (< start-of-message start-of-window))\r
 220 > > +     ((= start-of-message (point))\r
 221 > > +      ;; The cursor is at the start of the current message - move to\r
 222 > > +      ;; the previous open message.\r
 223 > > +      (notmuch-show-previous-open-message))\r
 224 > \r
 225 > This will jump to the beginning of the previous message if I'm at the\r
 226 > beginning of a message.  I would expect rewind to show me the end of\r
 227 > the previous message in this case.\r
 228 \r
 229 Agreed.  I would like to see this case move back one screenful of text or\r
 230 to the previous beginning-of-message, whichever is shorter.  Something\r
 231 like this should do the trick (replacing the notmuch-show-prev-msg call):\r
 232 \r
 233 (let ((last-msg-point (save-excursion\r
 234                               (notmuch-show-goto-message-previous)\r
 235                               (point))))\r
 236         (scroll-down)\r
 237         (if (> last-msg-point (window-start))\r
 238             (goto-char last-msg-point)\r
 239           (goto-char (window-start)))\r
 240         (notmuch-show-message-adjust))\r
 241 \r
 242 Thanks,\r
 243 \r
 244 -- \r
 245 Aaron Ecay\r