From: David Bremner <david@tethera.net>
To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/3] contrib: pick: remove hack
	notmuch-pick-show-match-message-with-wait
In-Reply-To: <1372582512-10777-3-git-send-email-markwalters1009@gmail.com>
References: <1372582512-10777-1-git-send-email-markwalters1009@gmail.com>
	<1372582512-10777-3-git-send-email-markwalters1009@gmail.com>
User-Agent: Notmuch/0.15.2+193~gb7df509 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Sun, 30 Jun 2013 13:30:42 -0300
Message-ID: <87li5rtvbh.fsf@zancas.localnet>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

Mark Walters <markwalters1009@gmail.com> writes:

> +(defvar notmuch-pick-open-target nil)
> +(make-variable-buffer-local 'notmuch-pick-open-target)

What do people think about adding a code style suggestion/requirement
for elisp that all variables have docstrings, even if intended for
internal use?  It's true the existing code doesn't really meet this
standard.

>  (defvar notmuch-pick-buffer-name nil)
>  (make-variable-buffer-local 'notmuch-pick-buffer-name)
>  ;; This variable is the window used for the message pane. It is set
> @@ -349,8 +351,8 @@ Does NOT change the database."
>    (notmuch-pick (notmuch-search-find-thread-id)
>                  notmuch-search-query-string
>  		nil
> -                (notmuch-prettify-subject (notmuch-search-find-subject)))
> -  (notmuch-pick-show-match-message-with-wait))
> +                (notmuch-prettify-subject (notmuch-search-find-subject))
> +		t))

I think my previous complaint can be reformulated as (essentially) both
notmuch-pick and notmuch-pick-open-target could use (better) docstrings.

As you say, the hack removed is quite horrible, so I'd be willing to
merge the patches anyway. OTOH, more documentation might make it so that
more than one person can understand the notmuch-pick code.

d