[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: ispell.el, flyspell.el: better ispell/aspell switching
From: |
Stefan Monnier |
Subject: |
Re: ispell.el, flyspell.el: better ispell/aspell switching |
Date: |
Wed, 16 Apr 2008 11:21:33 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) |
>> - 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
- ispell.el, flyspell.el: better ispell/aspell switching, Agustin Martin, 2008/04/04
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Agustin Martin, 2008/04/15
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Stefan Monnier, 2008/04/15
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Jason Rumney, 2008/04/15
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Agustin Martin, 2008/04/16
- Re: ispell.el, flyspell.el: better ispell/aspell switching,
Stefan Monnier <=
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Agustin Martin, 2008/04/17
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Stefan Monnier, 2008/04/17
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Agustin Martin, 2008/04/21
- Re: ispell.el, flyspell.el: better ispell/aspell switching, Stefan Monnier, 2008/04/23