From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id BB312431FB6 for ; Thu, 26 Jan 2012 22:52:57 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uYhrA-0ZZtbu for ; Thu, 26 Jan 2012 22:52:57 -0800 (PST) Received: from mail-we0-f181.google.com (mail-we0-f181.google.com [74.125.82.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id CAE12431FAE for ; Thu, 26 Jan 2012 22:52:56 -0800 (PST) Received: by werb10 with SMTP id b10so1134386wer.26 for ; Thu, 26 Jan 2012 22:52:54 -0800 (PST) Received: by 10.216.25.10 with SMTP id y10mr2391500wey.55.1327647174161; Thu, 26 Jan 2012 22:52:54 -0800 (PST) Received: from hotblack-desiato.hh.sledj.net (host81-149-164-25.in-addr.btopenworld.com. [81.149.164.25]) by mx.google.com with ESMTPS id d9sm9618984wiy.2.2012.01.26.22.52.52 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 26 Jan 2012 22:52:53 -0800 (PST) Received: by hotblack-desiato.hh.sledj.net (Postfix, from userid 30000) id C2537A02BA; Fri, 27 Jan 2012 06:52:50 +0000 (GMT) To: Dmitry Kurochkin , notmuch@notmuchmail.org Subject: Re: [PATCH 0/2] re-enable line wrapping and add some header bling In-Reply-To: <87ty3iqvyq.fsf@gmail.com> References: <1327565871-19729-1-git-send-email-dme@dme.org> <87ty3iqvyq.fsf@gmail.com> User-Agent: Notmuch/0.11+122~g97bdcf6 (http://notmuchmail.org) Emacs/24.0.92.1 (x86_64-pc-linux-gnu) From: David Edmondson Date: Fri, 27 Jan 2012 06:52:46 +0000 Message-ID: MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Jan 2012 06:52:57 -0000 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Thu, 26 Jan 2012 20:17:49 +0400, Dmitry Kurochkin 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. >=20 > 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). >=20 > 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. 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." ? --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEARECAAYFAk8iSb4ACgkQaezQq/BJZRadqgCeJwLfZs6Y62Io1cbhzpuumbTP mOcAn3tmgIRrwUOKvlMnpFhgwxghIYF4 =RZOR -----END PGP SIGNATURE----- --=-=-=--