From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: ispell.el, flyspell.el: better ispell/aspell switching Date: Wed, 16 Apr 2008 11:21:33 -0400 Message-ID: References: <20080404120217.GA7503@agmartin.aq.upm.es> <20080416094933.GA4876@agmartin.aq.upm.es> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1208359892 30050 80.91.229.12 (16 Apr 2008 15:31:32 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 16 Apr 2008 15:31:32 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Apr 16 17:32:04 2008 connect(): Connection refused Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1Jm9Sm-0004hl-GF for ged-emacs-devel@m.gmane.org; Wed, 16 Apr 2008 17:22:20 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jm9S7-0002Lk-Hv for ged-emacs-devel@m.gmane.org; Wed, 16 Apr 2008 11:21:39 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Jm9S4-0002Lb-3r for emacs-devel@gnu.org; Wed, 16 Apr 2008 11:21:36 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Jm9S3-0002LP-Ik for emacs-devel@gnu.org; Wed, 16 Apr 2008 11:21:35 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jm9S3-0002LL-FR for emacs-devel@gnu.org; Wed, 16 Apr 2008 11:21:35 -0400 Original-Received: from ironport2-out.pppoe.ca ([206.248.154.182] helo=ironport2-out.teksavvy.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Jm9S3-0005tN-DJ for emacs-devel@gnu.org; Wed, 16 Apr 2008 11:21:35 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Aq8EAHi2BUhMCqsI/2dsb2JhbACBYK0+ X-IronPort-AV: E=Sophos;i="4.25,664,1199682000"; d="scan'208";a="18653936" Original-Received: from smtp.pppoe.ca (HELO smtp.teksavvy.com) ([65.39.196.238]) by ironport2-out.teksavvy.com with ESMTP; 16 Apr 2008 11:21:33 -0400 Original-Received: from pastel.home ([76.10.171.8]) by smtp.teksavvy.com (Internet Mail Server v1.0) with ESMTP id WTL55233; Wed, 16 Apr 2008 11:21:33 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 336858DD0; Wed, 16 Apr 2008 11:21:33 -0400 (EDT) In-Reply-To: <20080416094933.GA4876@agmartin.aq.upm.es> (Agustin Martin's message of "Wed, 16 Apr 2008 11:49:33 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) X-detected-kernel: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:95354 Archived-At: >> - ispell-spellchecker-init-pre-hook is not documented. What are >> distro-override-dicts-alist and distro-fallback-dicts-alist? > The hook is intended for easier interaction of distro maintainers wrt the > dictionary alist. E.g., in Debian ispell and aspell dictionary packages > provide info about the dictionary that might or not be in > original ispell-dictionary-alist. That hook is intended to allow easy > modification of both distro-override-dicts-alist and > distro-fallback-dicts-alist. These have the same format as > ``ispell-dictionary-alist'' and are (very poorly) documented when > defined in the 'let', and allow two levels of overriding, > ``distro-override-dicts-alist'' will override even dicts in > ``found-dicts-alist'' (currently the alist of parsed aspell dicts and > associated properties if spellchecker is aspell). Put here just in > case is ever needed, but I am currently not using it in Debian. In what kind of circumstance would this be needed? > ``distro-fallback-dicts-alist'' will just override (or complement if > originally not present) initial ``ispell-dictionary-alist'' values > (those that are in ``base-dicts'' in the function), so an entry can be > fixed without patching ispell.el or waiting for a new emacs version > be released. Can't this be done by: 1 - remove autoloads on ispell-dictionary-alist-N 2 - merge ispell-dictionary-alist-N into ispell-base-dictionary-alist 3 - let Debian use after-eval-hook to adjust ispell-base-dictionary-alist > For instance, this is the relevant part in debian-ispell.el > --------------------------- > (defun debian-ispell-initialize-dicts-alist () > (setq distro-fallback-dicts-alist > (if ispell-really-aspell > debian-aspell-only-dictionary-alist > debian-ispell-only-dictionary-alist))) Why does Debian need to do that? I.e. What kind of changes does this effect? Why doesn't ispell.el get it right on its own? > (add-hook 'ispell-spellchecker-init-pre-hook > 'debian-ispell-initialize-dicts-alist) > ``debian-aspell-only-dictionary-alist'' and > ``debian-ispell-only-dictionary-alist'' being built after the info > provided by the package maintainers. I must say that I generally prefer if there aren't any hooks and when we don't use dynamic scoping, so I'm not thrilled about this code. I understand that there might be a need for some hook, but I'd rather it doesn't use dynamic scoping. Could you get away with a normal hook run at the end of ispell-set-spellchecker-params, which would work by modifying ispell-dictionary-alist? > Should I document the hook just in the function, or there is a better place? Add a defvar for the hook, where you can document it. >> - (setq ispell-last-program-name ispell-program-name) should be done >> right after checking if they're equal, not at the end of the function. > I usually do this kind of things after the work is done, so is only changed > if nothing wrong happened. In case of an error, I'd rather avoid doing it over-and-over-and-over-and-over ... also it's good to have it all in a single place. But I won't insist. >> - (defvar ispell-aspell-dictionary-alist...) should be before the first >> use of that variable. > I just put it in the aspell stuff section, but I agree that is better as you > propose, before (ispell-set-spellchecker-params). I should probably move the > ispell-set-spellchecker-params stuff below the aspell stuff, so each has an > own block. Either way is fine by me. >> BTW (this is unrelated to this patch, but I just noticed it while >> looking at the code): why are ispell-dictionary-alist* autoloaded? >> I just tried to remove the autoloads on them and I couldn't notice any >> negative effect. > As you pointed out, presumably because the way menus were previously built > required it. > While we are at this, since ``ispell-dictionary-alist'' is in my patch built > from its components in (ispell-set-spellchecker-params), its original > definition may be simplified to just a > (defvar ispell-dictionary-alist nil > " ..... The long description of ispell-dictionary-list ... > follows > ...." > probably adding a coment like > ;; Its actual value will be set in the (ispell-set-spellchecker-params) > ;; function Yes, I reached the exact same conclusion. Stefan