DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma;
	h=mime-version:sender:in-reply-to:references:date
	:x-google-sender-auth:message-id:subject:from:to:cc:content-type;
	b=HdF2T5Vz4G2TdLdr8CAFYspgvBygLtOLR0wYfsElzYd2cTdbuqBO2FI+3zwJ6NCG9l
	YCEaxtvX5LXWNeVrDxjQq2scw78dpOF+TUu7ncl7RjjU21OSnX6xQliIsXece+H4afd9
	ICaiVIh9k2QPGiKPWCnBnNFzERlPuUeVcjcbc=
MIME-Version: 1.0
Sender: amdragon@gmail.com
In-Reply-To: <1307921480-17130-3-git-send-email-daniel.schoepe@googlemail.com>
References: <1307921480-17130-1-git-send-email-daniel.schoepe@googlemail.com>
	<1307921480-17130-3-git-send-email-daniel.schoepe@googlemail.com>
Date: Mon, 13 Jun 2011 13:23:20 -0400
Message-ID: <BANLkTinfRrH=172YOy45zSfE-PHo3S=U_A@mail.gmail.com>
Subject: Re: [PATCH 2/3] emacs: Add thread-outline functionality
From: Austin Clements <amdragon@mit.edu>
To: Daniel Schoepe <daniel.schoepe@googlemail.com>
Content-Type: text/plain; charset=ISO-8859-1
Cc: notmuch@notmuchmail.org
Precedence: list

Have you tried post-command-hook?  It's overkill, but if you can very
quickly check that you don't have to do anything, the overhead should
be negligible.  It doesn't have the strange point-entered behavior,
but still has a marked advantage over zero-duration idle timers
because you can register your hook for just the outline and show
buffers and it won't get called after actions in other buffers.

I do have one overall structural concern with this patch.  This might
be addressed along with better synchronization between the show and
outline buffers, but my concern is what happens when you switch
between buffers (in particular, multiple show buffers).  I think the
behavior I would expect is that if I switch to another show buffer,
the outline would update to reflect that other show buffer.  If I
switch to a non-show buffer, I would probably expect the outline
window to go away.  Thoughts?

 (defun notmuch-kill-this-buffer ()
   "Kill the current buffer."
   (interactive)
+  ;; if we are in a notmuch-show buffer, kill the associated outline
buffer, if any
+  (when (eq major-mode 'notmuch-show-mode)
+    (let ((outline-buf (get-buffer (notmuch-show-outline-buffer-name))))
+      (when outline-buf
+	(kill-buffer outline-buf))))
   (kill-buffer (current-buffer)))

Just killing the buffer leads to some strange behavior for me.  If I'm
in a show buffer, then hit 'o' and then 'q', my frame is still split
into two windows, one of which shows where I came from, and the other
shows some other buffer (probably the next furthest back in the LRU;
usually notmuch-hello or another search).

Also, would it work better to store the outline buffer object as a
buffer-local variable in the show buffer?  Then you wouldn't have to
go hunting for it.

+(defvar notmuch-outline-button-map
+  (let ((map (copy-keymap button-map)))
+    (define-key map (kbd "<mouse-1>") 'push-button)
+    map)
+  "Keymap used for buttons in thread outlines.")

I don't think you need this.  Does it work if you just add
'follow-link t to the make-button call?

+(defun notmuch-goto-marker (m)
+"Open corresponding buffer and go to marker position in another window."
+  (switch-to-buffer-other-window (marker-buffer m))
+  (goto-char (marker-position m)))

Overkill?

+(defcustom notmuch-outline-format
+  '(("author" . "%s")
+    "-"
+    ("reldate" . "%s"))
+  "Format in which thread-outline entries are displayed
+
+The following fields are supported: date, reldate, author,
+subject.  The list can also contain strings as elements which
+will be printed literally.  This variable can also be a function
+that will be given the message as returned by
+`notmuch-show-get-message-properties' and should return a
+string."

I would recommend a format closer to Emacs' standard format lines like
mode-line-format and header-line-format, or, more likely, some
compatible subset thereof.  In addition to being familiar, those have
the advantage of being recursive, symbolic, and able to embed
arbitrary computations within the convenience of the rest of the
formatter.  One trick I've found works really well in the past is to
let-bind things like `author' and `reldate' in the format function;
this combines naturally with expanding symbols to their values like
mode-line-format does.

+(defun notmuch-show-has-outline ()
+  "Returns non-nil if there is an outline for the current thread."
+  (get-buffer (notmuch-show-outline-buffer-name)))

Unused?

+(defun notmuch-outline-message ()
+  "Outline the message under the point.
+
+Expects the point to be on the beginning of the first line of the message."
+  (lexical-let* ((extent (notmuch-show-message-extent))
+                (buffer-name (notmuch-show-outline-buffer-name))

This would probably be simpler if you just passed the outline buffer
as an argument to `notmuch-outline-message', rather than
reconstructing the buffer name.

+      ;; this is not very robust if the output of notmuch-show changes
+      (while (string-equal (thing-at-point 'char) " ")
+       (incf indentation)
+       (forward-char))

Rather than parsing the notmuch-show buffer, would it make more sense
for notmuch-show to add some property indicating the reply level,
which this could simply read?  Counting spaces seems like asking for
trouble.

+      (loop for i from 1 to indentation do
+           (princ " ")) ;; somewhat ugly

If you move the "(with-current-buffer standard-output" up, this could
simply be indent-to (and the princ's could be replaced with the less
unusual `insert').