From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Agustin Martin Newsgroups: gmane.emacs.devel Subject: Re: ispell.el, flyspell.el: better ispell/aspell switching Date: Mon, 21 Apr 2008 19:15:19 +0200 Message-ID: <20080421171519.GA7078@agmartin.aq.upm.es> References: <20080404120217.GA7503@agmartin.aq.upm.es> <20080416094933.GA4876@agmartin.aq.upm.es> <20080417175637.GA4277@agmartin.aq.upm.es> NNTP-Posting-Host: dough.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="liOOAslEiF7prFVr" X-Trace: ger.gmane.org 1208798283 17469 80.91.229.10 (21 Apr 2008 17:18:03 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 21 Apr 2008 17:18:03 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Apr 21 19:37:20 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by dough.gmane.org with esmtp (Exim 4.50) id 1Jnzwm-0005DQ-IJ for ged-emacs-devel@m.gmane.org; Mon, 21 Apr 2008 19:37:05 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jnzdv-0004Oi-N1 for ged-emacs-devel@m.gmane.org; Mon, 21 Apr 2008 13:17:27 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Jnzdd-0004Jb-Qg for emacs-devel@gnu.org; Mon, 21 Apr 2008 13:17:09 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JnzdY-0004Ha-Ba for emacs-devel@gnu.org; Mon, 21 Apr 2008 13:17:09 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JnzdX-0004HR-Sp for emacs-devel@gnu.org; Mon, 21 Apr 2008 13:17:04 -0400 Original-Received: from euler.ccupm.upm.es ([138.100.4.67] helo=smtp.upm.es) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JnzdV-0002RM-VS for emacs-devel@gnu.org; Mon, 21 Apr 2008 13:17:03 -0400 Original-Received: from mala.aq.upm.es (Agmartin.aq.upm.es [138.100.41.131]) by smtp.upm.es (8.13.8/8.13.8/euler-005) with ESMTP id m3LHFJ2k013280; Mon, 21 Apr 2008 19:15:19 +0200 Original-Received: by mala.aq.upm.es (Postfix, from userid 1000) id A455732643; Mon, 21 Apr 2008 19:15:19 +0200 (CEST) Mail-Followup-To: emacs-devel@gnu.org Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17+20080114 (2008-01-14) X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) 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:95655 Archived-At: --liOOAslEiF7prFVr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Apr 17, 2008 at 09:38:51PM -0400, Stefan Monnier wrote: > I think it's perfectly fine to include it in ispell.el (it doesn't have > to be limited to the use of distro's tho, so I'd rather not use the word > "distro". As a general rule, variables and functions should take names > that describe what they do rather than in what context they'll be used) Thanks, I now notice that this may also be used systemwide by sysadmins for e.g., non-standard dicts, so removing the 'distro' word completely and using ``ispell-initialize-spellchecker-hook'' instead. > > > I think this is better done as currently, just in the middle, that > > is, after parsed aspell dicts, but before base-dicts. You do not know where > > things come from if things are done at the end, and if you need to check it > > you may end reusing half of the function. > > Is it ever necessary/important to distinguish whether it comes from the > base or fom the parsed dicts? I think that only in the function. > > If the problem is about using a hook there, something like > > No, the problem is the dynamically scoped variables. But if there's no > easy way to do without them, it's OK to use such things. Just use more > descriptive names, and be sure to document them in the hook's docstring. Thanks for all the suggestions, I am attaching updated patches. Feel free to change if you think there are better names or ways of doing things. ispell.el: ========== (ispell-set-spellchecker-params): New function to make sure right params and dictionary alists are used after spellchecker changes. ``ispell-aspell-dictionary-alist'', ``ispell-last-program-name'', ``ispell-initialize-spellchecker-hook'' New variables and hook: used by (ispell-set-spellchecker-params) (ispell-find-aspell-dictionaries) Modified to use ``ispell-aspell-dictionary-alist'' (ispell-maybe-find-aspell-dictionaries) Removed. Calls replaced by (ispell-set-spellchecker-params) calls. (ispell-have-aspell-dictionaries) Removed. flyspell.el: ============ (ispell-maybe-find-aspell-dictionaries) Calls replaced by (ispell-set-spellchecker-params) calls. -- Agustin --liOOAslEiF7prFVr Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="ispell.el_ispell-set-spellchecker-params.2.diff" --- ispell.el.orig 2008-03-28 12:55:25.000000000 +0100 +++ ispell.el 2008-04-21 12:47:47.000000000 +0200 @@ -881,18 +881,9 @@ ;; Make ispell.el work better with aspell. -(defvar ispell-have-aspell-dictionaries nil - "Non-nil if we have queried Aspell for dictionaries at least once.") - -(defun ispell-maybe-find-aspell-dictionaries () - "Find Aspell's dictionaries, unless already done." - (when (and (not ispell-have-aspell-dictionaries) - (condition-case () - (progn (ispell-check-version) t) - (error nil)) - ispell-really-aspell - ispell-aspell-supports-utf8) - (ispell-find-aspell-dictionaries))) +(defvar ispell-aspell-dictionary-alist nil + "An alist of parsed aspell dicts and associated parameters. +Internal use.") (defun ispell-find-aspell-dictionaries () "Find Aspell's dictionaries, and record in `ispell-dictionary-alist'." @@ -915,14 +906,13 @@ (dolist (dict ispell-dictionary-alist) (unless (assoc (car dict) found) (setq found (nconc found (list dict))))) - (setq ispell-dictionary-alist found) + (setq ispell-aspell-dictionary-alist found) ;; Add a default entry (let* ((english-dict (assoc "en" ispell-dictionary-alist)) (default-dict (cons nil (or (cdr english-dict) (cdr (car ispell-dictionary-alist-1)))))) - (push default-dict ispell-dictionary-alist)) - (setq ispell-have-aspell-dictionaries t))) + (push default-dict ispell-aspell-dictionary-alist)))) (defvar ispell-aspell-data-dir nil "Data directory of Aspell.") @@ -1004,11 +994,74 @@ (push (cons aliasname (cdr realdict)) alist)))))) alist)) +;; Set params according to the selected spellchecker + +(defvar ispell-last-program-name nil + "Last value of ispell-program name. Internal use.") + +(defvar ispell-initialize-spellchecker-hook nil + "Actions to be taken on spellchecker initialization. +This hook is run when an spellchecker is used for the first +time, before ``ispell-dictionary-alist'' is set. Is intended for +sysadmins to override entries in the ispell.el base dictionary-alist +by putting those overrides in a ``ispell-base-dicts-override-alist'' +alist with same format as ``ispell-dictionary-alist''. This alist +will not override the auto-detected values if a recent aspell is +used along with emacs.") + +(defun ispell-set-spellchecker-params () + "Initialize some spellchecker parameters when changed or first used." + (unless (eq ispell-last-program-name ispell-program-name) + (setq ispell-last-program-name ispell-program-name) + (ispell-kill-ispell t) + (if (and (condition-case () + (progn + (setq ispell-library-directory (ispell-check-version)) + t) + (error nil)) + ispell-really-aspell + ispell-aspell-supports-utf8 + ;; xemacs does not like [:alpha:] regexps + (string-match "^[[:alpha:]]+$" "abcde")) + (unless ispell-aspell-dictionary-alist + (ispell-find-aspell-dictionaries))) + + ;; Substitute ispell-dictionary-alist with the list of dictionaries + ;; corresponding to the given spellchecker. If a recent aspell, use + ;; the list of really installed dictionaries and add to it elements + ;; of the original list that are not present there. Allow distro info. + (let ((base-dicts-alist + (append ispell-dictionary-alist-1 ispell-dictionary-alist-2 + ispell-dictionary-alist-3 ispell-dictionary-alist-4 + ispell-dictionary-alist-5 ispell-dictionary-alist-6)) + (found-dicts-alist + (if (and ispell-really-aspell + ispell-aspell-supports-utf8) + ispell-aspell-dictionary-alist + nil)) + ispell-base-dicts-override-alist ; Override only base-dicts-alist + all-dicts-alist) + + (run-hooks 'ispell-initialize-spellchecker-hook) + + ;; Add dicts to ``ispell-dictionary-alist'' unless already present. + (dolist (dict (append found-dicts-alist + ispell-base-dicts-override-alist + base-dicts-alist)) + (unless (assoc (car dict) all-dicts-alist) + (add-to-list 'all-dicts-alist dict))) + (setq ispell-dictionary-alist all-dicts-alist)))) + + (defun ispell-valid-dictionary-list () "Returns a list of valid dictionaries. The variable `ispell-library-directory' defines the library location." - ;; If Ispell is really Aspell, query it for the dictionary list. - (ispell-maybe-find-aspell-dictionaries) + ;; Initialize variables and dictionaries alists for desired spellchecker. + ;; Make sure ispell.el is loaded to avoid some autoload loops in xemacs + ;; (and may be others) + (if (featurep 'ispell) + (ispell-set-spellchecker-params)) + (let ((dicts (append ispell-local-dictionary-alist ispell-dictionary-alist)) (dict-list (cons "default" nil)) name load-dict) @@ -1566,7 +1619,7 @@ (ispell-region (region-beginning) (region-end))) (continue (ispell-continue)) (t - (ispell-maybe-find-aspell-dictionaries) + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists (ispell-accept-buffer-local-defs) ; use the correct dictionary (let ((cursor-location (point)) ; retain cursor location (word (ispell-get-word following)) @@ -2602,7 +2655,7 @@ (mapcar 'list (ispell-valid-dictionary-list))) nil t) current-prefix-arg)) - (ispell-maybe-find-aspell-dictionaries) + (ispell-set-spellchecker-params) ; Initilize variables and dicts alists (unless arg (ispell-buffer-local-dict 'no-reload)) (if (equal dict "default") (setq dict nil)) ;; This relies on completing-read's bug of returning "" for no match @@ -2653,7 +2706,7 @@ Return nil if spell session is quit, otherwise returns shift offset amount for last line processed." (interactive "r") ; Don't flag errors on read-only bufs. - (ispell-maybe-find-aspell-dictionaries) + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists (if (not recheckp) (ispell-accept-buffer-local-defs)) ; set up dictionary, local words, etc. (let ((skip-region-start (make-marker)) --liOOAslEiF7prFVr Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="flyspell.el_ispell-set-spellchecker-params.diff" --- flyspell.el.orig 2008-03-28 12:55:25.000000000 +0100 +++ flyspell.el 2008-03-28 12:58:02.000000000 +0100 @@ -578,7 +578,7 @@ ;;*---------------------------------------------------------------------*/ (defun flyspell-mode-on () "Turn Flyspell mode on. Do not use this; use `flyspell-mode' instead." - (ispell-maybe-find-aspell-dictionaries) + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists (setq ispell-highlight-face 'flyspell-incorrect) ;; local dictionaries setup (or ispell-local-dictionary ispell-dictionary @@ -1016,6 +1016,7 @@ (defun flyspell-word (&optional following) "Spell check a word." (interactive (list ispell-following-word)) + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists (save-excursion ;; use the correct dictionary (flyspell-accept-buffer-local-defs) @@ -1531,7 +1532,7 @@ ;; this is done, we can start checking... (if flyspell-issue-message-flag (message "Checking region...")) (set-buffer curbuf) - (ispell-check-version) + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists ;; Local dictionary becomes the global dictionary in use. (setq ispell-current-dictionary (or ispell-local-dictionary ispell-dictionary)) @@ -1590,6 +1591,7 @@ (defun flyspell-region (beg end) "Flyspell text between BEG and END." (interactive "r") + (ispell-set-spellchecker-params) ; Initialize variables and dicts alists (if (= beg end) () (save-excursion --liOOAslEiF7prFVr--