Hi David. On Thu, 26 Jan 2012 08:17:49 +0000, David Edmondson <dme at 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 at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch