From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 0/2] re-enable line wrapping and add some header bling
In-Reply-To: <1327565871-19729-1-git-send-email-dme@dme.org>
References: <1327565871-19729-1-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.11+116~ge6e10b8 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 26 Jan 2012 20:17:49 +0400
Message-ID: <87ty3iqvyq.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

Hi David.

On Thu, 26 Jan 2012 08:17:49 +0000, David Edmondson <dme@dme.org> wrote:
> By default, re-enable `visual-line-mode' in `notmuch-show-mode'. Do it
> via a hook so that purists (ahem) can turn it off.
> 
> Add some more processing of headers to make them look nice. Do it via
> hooks so that unbelievers can turn it off.
> 

I did not review the code, but here is a general comment for both
patches (but especially for the first one).  It would be nice to have a
more detailed documentation for hooks.  Docstring like "Enable Visual
Line mode." for a function named `notmuch-show-turn-on-visual-line-mode'
is near useless.  It is quite obvious that the function enables
visual-line-mode from it's name.  And it does not give any information
on why would someone actually want to use it.  I do not remember what
visual-line-mode is exactly, so to understand whether this hook is
actually useful for me, I have to read visual-line-mode docs, think
about how it helps in notmuch-show, read some code, perhaps, etc.  I
would argue that since the hook itself is trivial, the main point in
having it is to provide a clearly documented solution for a common
problem for those who do not know how to solve this problem right away.
Currently, those who know what visual-line-mode is do not need this
hook, because they can easily write their own, and those who do not know
what visual-line-mode is can not use this hook, because it says nothing
about why it is actually useful.

Also, in addition to better docs, I would rename
`notmuch-show-turn-on-visual-line-mode' to something that reflects what
it does from user POV (like the other two hooks).

Though, the fact that the hook is enabled by default makes the above
arguments less important, I guess.

Regards,
  Dmitry

> David Edmondson (2):
>   emacs: Re-enable line wrapping in `notmuch-show-mode'.
>   emacs: Add more processing of displayed headers.
> 
>  emacs/notmuch-show.el |   50 +++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 42 insertions(+), 8 deletions(-)
> 
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch