[PATCH 0/2] re-enable line wrapping and add some header bling

[Jani Nikula]

On Fri, 27 Jan 2012 06:52:46 +0000, David Edmondson <dme at dme.org> wrote:
> On Thu, 26 Jan 2012 20:17:49 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > 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.
> 
> I have a bunch of somewhat conflicting thoughts about this:
>   - Being able to configure the behaviour without having to change the
>     core code is good, so implementing behaviour using hook functions is
>     useful.
>   - Things should behave well in the default configuration, so most of
>     the hook functions are enabled by default.
>   - Everything can't be hook functions, so there's a balance to be made
>     between implementing things as in-line code and via hook functions.
>   - Most users shouldn't need to modify any of the hooks.
>   - Documentation that explains what a hook function is about is
>     obviously good.
>   - Documenting something that is external to notmuch can be both
>     wasteful and risky.
> 
>     Wasteful because such documentation typically already exists and
>     risky because the precise behaviour of external components is not
>     under our control.
> 
>     For example, the documentation for `visual-line-mode' is both
>     concise and good, so there's little point in repeating it and, of
>     course, the exact details of what `visual-line-mode' does can be
>     changed by the Emacs developers. One would expect that they would
>     update the documentation for `visual-line-mode' in such situations,
>     which would leave any cloned documentation in an incorrect state.
> 
> Hence, I would probably take issue with your statement:
> 
> > 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.
> 
> That's not the intention of the hook functions under discussion
> here. They are hook functions so that a curious and interested user can
> make a change based on some aesthetic preference. The are not about
> solving problems with the default configuration.
> 
> I think that `turn-on-visual-line-mode' (or the package local derivative
> of it that was chosen) is precisely the right name for a function that
> enables `visual-line-mode'. It describes perfectly and succinctly what
> the function does and provides a pointer to follow to find out more (the
> documentation for `visual-line-mode') without a user even having to
> examine the documentation of the function itself.

Agree completely.

Concrete suggestion:

+(defun notmuch-show-turn-on-visual-line-mode ()
+  "Enable `visual-line-mode'."
+  (visual-line-mode t))

To provide a link to the visual-line-mode documentation.

> All of that said, I'd agree that the documentation of
> `notmuch-show-indent-continuations' could have been better. How about:
>   "Indent any continuation lines in the current headers."
> ?

Sounds good.


BR,
Jani.