Re: [PATCH v2 2/3] contrib: pick: remove hack notmuch-pick-show-match-message-with-wait

Subject: Re: [PATCH v2 2/3] contrib: pick: remove hack notmuch-pick-show-match-message-with-wait

Date: Sun, 30 Jun 2013 13:30:42 -0300

To: Mark Walters, notmuch@notmuchmail.org

Cc:

From: David Bremner


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


Thread: