bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Akim Demaille]

Hi all,

I have recently upgraded my Emacs from 22.2.1 to 24.2.1, and there is
a feature I lost: I could use ispell-personal-dictionary to specify a
per-document *local* (i.e., relative to the directory of the file)
dictionary.

Consider the following use case: several documents (LaTeX for that
matter), with many uncommon words in common. Instead of having a
bazillion of (duplicated) "LocalWords:" in all my documents, I have:

| %%% Local Variables:
| %%% mode: latex
| %%% coding: utf-8
| %%% ispell-dictionary: "american"
| %%% ispell-personal-dictionary: "../en.dict"
| %%% TeX-master: t
| %%% fill-column: 76
| %%% End:

in each file.

This used to work well, i.e., aspell was given the right path to
en.dict.  With the current version of Emacs, "../en.dict" seems to be
resolved relatively to the directory from which Emacs was run, instead
of relatively to the document.

I do not want to force a full path to the dictionary, because the
documents are under VCS, and the dictionary is shared between the
authors.

I have run the following experiment, on a file with these local
variables:

| %%% Local Variables:
| %%% mode: latex
| %%% coding: utf-8
| %%% ispell-dictionary: "american"
| %%% ispell-personal-dictionary: "en.dict"
| %%% TeX-master: t
| %%% fill-column: 76
| %%% End:

Of course, what is expected is that en.dict is a local file, "in ./"
relatively to the file with these local variables.

With Emacs 22, I have the expected behavior:

| $ /usr/bin/emacs --version
| GNU Emacs 22.1.1
| Copyright (C) 2007 Free Software Foundation, Inc.
| GNU Emacs comes with ABSOLUTELY NO WARRANTY.
| You may redistribute copies of Emacs
| under the terms of the GNU General Public License.
| For more information about these matters, see the file named COPYING.

| $ /usr/bin/emacs f.tex
                       -- (here, I "M-x ispell", and "C-z" Emacs)
| 
| zsh: suspended  /usr/bin/emacs f.tex
| $ ps aux | grep aspell
| […] /opt/local/bin/aspell -a -m -d en_US --encoding=utf-8 -p /Users/akimd/src/lrde/cours-mob/mob1/src/lecture_2/en.dict

As you can see, the location of the en.dict is correct (f.tex is
indeed in /Users/akimd/src/lrde/cours-mob/mob1/src/lecture_2).

Now with Emacs 24:

| $ emacs --version
| GNU Emacs 24.2.1
| Copyright (C) 2012 Free Software Foundation, Inc.
| GNU Emacs comes with ABSOLUTELY NO WARRANTY.
| You may redistribute copies of Emacs
| under the terms of the GNU General Public License.
| For more information about these matters, see the file named COPYING.

| $ emacs mob1/src/lecture_2/f.tex
| 
| zsh: suspended  emacs mob1/src/lecture_2/f.tex
| $ ps aux | grep aspell
| […] /opt/local/bin/aspell -a -m -d en_US -p /Users/akimd/en.dict --encoding=utf-8

this time its completely wrong.


The previous behavior was very useful, as it made it possible for a
repository of documents to share a dictionary (more useful than
per-document Local Words).

Yes, there is the "eval" option, but that's working around a bug imho,
with the nasty effect that each time the document is opened, you have
to validate the action.

If "personal" was really meant to be the user's personal data, then it
was easy to use "~/" to make it clear, and it is still easy today.  On
the other hand, the way the dictionary name is turned absolute in 24.2
forbids the previous useful approach.

Or have I missed something?

Thanks in advance!

----------------------------------------------------------------------

In GNU Emacs 24.2.1 (x86_64-apple-darwin12.2.0)
 of 2012-11-26 on mcote-eth.lrde.epita.fr
Configured using:
 `configure '--prefix=/opt/local' '--without-x' '--without-dbus'
 '--without-gconf' '--without-libotf' '--without-m17n-flt'
 '--without-gpm' '--without-gnutls' '--without-xml2' '--infodir'
 '/opt/local/share/info/emacs' 'CC=/usr/bin/clang' 'CFLAGS=-pipe -O2
 -arch x86_64' 'LDFLAGS=-L/opt/local/lib -Wl,-no_pie -arch x86_64'
 'CPPFLAGS=-I/opt/local/include''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message idna format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail regexp-opt rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils time-date auctex-autoloads
tex-site info tool-bar easymenu package tabulated-list uniquify advice
help-fns advice-preload ido cus-start cus-load ediff-hook vc-hooks
lisp-float-type lisp-mode register page menu-bar rfn-eshadow timer
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer loaddefs button faces cus-face files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process multi-tty emacs)

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Glenn Morris]

Akim Demaille wrote:

> I have recently upgraded my Emacs from 22.2.1 to 24.2.1, and there is
> a feature I lost: I could use ispell-personal-dictionary to specify a
> per-document *local* (i.e., relative to the directory of the file)
> dictionary.

Presumably due to 2010-09-02 changes to ispell-init-process for
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6143

Perhaps a file-name-absolute-p check for various variables could be
file-local could be added to that.

> | %%% ispell-dictionary: "american"
> | %%% ispell-personal-dictionary: "../en.dict"

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Agustin Martin]

On Wed, Nov 28, 2012 at 04:16:53PM -0500, Glenn Morris wrote:
> Akim Demaille wrote:
> 
> > I have recently upgraded my Emacs from 22.2.1 to 24.2.1, and there is
> > a feature I lost: I could use ispell-personal-dictionary to specify a
> > per-document *local* (i.e., relative to the directory of the file)
> > dictionary.
> 
> Presumably due to 2010-09-02 changes to ispell-init-process for
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6143
> 
> Perhaps a file-name-absolute-p check for various variables could be
> file-local could be added to that.
> 
> > | %%% ispell-dictionary: "american"
> > | %%% ispell-personal-dictionary: "../en.dict"

Actual problem happens in `ispell-start-process' where the string to be
passed to the spellchecker as personal dictionary is expanded,

(if ispell-current-personal-dictionary ; Use specified pers dict.
    (list "-p" (expand-file-name ispell-current-personal-dictionary)))

and is indeed affected by the changes in `default-dictionary' coming from
`ispell-init-process`.

I think the only reason for that `expand-file-name' is the tilde expansion
in non-UNIX systems. In UNIX systems tilde can be explicitly passed. 
Not sure if the directory {forward,back}slash is also related to this.

I am thinking about expanding `ispell-current-personal-dictionary' string
only if it contains a tilde prefix.

However I am too biassed to the UNIX world. Anyone at the e.g. win32 world
can provide info about possible problems with that?

-- 
Agustin

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Agustin Martin]

[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]

On Fri, Nov 30, 2012 at 06:11:56PM +0100, Agustin Martin wrote:
> On Wed, Nov 28, 2012 at 04:16:53PM -0500, Glenn Morris wrote:
> > Akim Demaille wrote:
> > 
> > > I have recently upgraded my Emacs from 22.2.1 to 24.2.1, and there is
> > > a feature I lost: I could use ispell-personal-dictionary to specify a
> > > per-document *local* (i.e., relative to the directory of the file)
> > > dictionary.
> > 
> > Presumably due to 2010-09-02 changes to ispell-init-process for
> > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6143
> > 
> > Perhaps a file-name-absolute-p check for various variables could be
> > file-local could be added to that.
> > 
> > > | %%% ispell-dictionary: "american"
> > > | %%% ispell-personal-dictionary: "../en.dict"
> 
> Actual problem happens in `ispell-start-process' where the string to be
> passed to the spellchecker as personal dictionary is expanded,
> 
> (if ispell-current-personal-dictionary ; Use specified pers dict.
>     (list "-p" (expand-file-name ispell-current-personal-dictionary)))
> 
> and is indeed affected by the changes in `default-dictionary' coming from
> `ispell-init-process`.
> 
> I think the only reason for that `expand-file-name' is the tilde expansion
> in non-UNIX systems. In UNIX systems tilde can be explicitly passed. 
> Not sure if the directory {forward,back}slash is also related to this.
> 
> I am thinking about expanding `ispell-current-personal-dictionary' string
> only if it contains a tilde prefix.

I am now playing with something different, preserve original value of
`default-directory' in `current-ispell-directory' variable and expand 
personal dictionary after it if no absolute path or tilde is used. 

I think this is a better approach, see attached patch.

Let me test this a bit more before committing. Feel free to test it too.

-- 
Agustin

[-- Attachment #2: ispell.el_honour-personal-dictionary-path.diff --]
[-- Type: text/x-diff, Size: 2226 bytes --]

--- ispell.el.orig	2012-11-30 17:07:14.641701686 +0100
+++ ispell.el	2012-11-30 18:50:08.725283277 +0100
@@ -2669,7 +2669,8 @@
            orig-args
            (if ispell-current-personal-dictionary ; Use specified pers dict.
                (list "-p"
-                     (expand-file-name ispell-current-personal-dictionary)))
+                     (expand-file-name ispell-current-personal-dictionary
+				       current-ispell-directory)))
            ;; If we are using recent aspell or hunspell, make sure we use the
            ;; right encoding for communication. ispell or older aspell/hunspell
            ;; does not support this.
@@ -2706,18 +2707,21 @@
   (let* (;; Basename of dictionary used by the spell-checker
 	 (dict-bname (or (car (cdr (member "-d" (ispell-get-ispell-args))))
 			 ispell-current-dictionary))
+	 ;; The directory where process was started.
+	 (current-ispell-directory default-directory)
+	 ;; The default directory for the process.
 	 ;; Use "~/" as default-directory unless using Ispell with per-dir
 	 ;; personal dictionaries and not in a minibuffer under XEmacs
 	 (default-directory
 	   (if (or ispell-really-aspell
 		   ispell-really-hunspell
 		   ;; Protect against bad default-directory
-		   (not (and (file-directory-p default-directory)
-			     (file-readable-p default-directory)))
+                   (not (and (file-directory-p current-ispell-directory)
+                             (file-readable-p current-ispell-directory)))
 		   ;; Ispell and per-dir personal dicts available
-		   (not (or (file-readable-p (concat default-directory
+		   (not (or (file-readable-p (concat current-ispell-directory
 						     ".ispell_words"))
-			    (file-readable-p (concat default-directory
+			    (file-readable-p (concat current-ispell-directory
 						     ".ispell_"
 						     (or dict-bname
 							 "default")))))
@@ -2725,7 +2729,7 @@
 		   (and (window-minibuffer-p)
 			(not (fboundp 'minibuffer-selected-window))))
 	       (expand-file-name "~/")
-	     (expand-file-name default-directory))))
+	     (expand-file-name current-ispell-directory))))
     ;; Check if process needs restart
     (if (and ispell-process
 	     (eq (ispell-process-status) 'run)

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Stefan Monnier]

> I am now playing with something different, preserve original value of
> `default-directory' in `current-ispell-directory' variable and expand
> personal dictionary after it if no absolute path or tilde is used.

IIRC a related issue is whether the ispell process can be kept (rather
than killed&restarted) when switching buffer.

For my use case, it's fairly important that the ispell process not be
killed&restarted unnecessarily and indeed most of my buffers can share
the same ispell process because they use no buffer-specific dictionary.


        Stefan

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Eli Zaretskii]

> Date: Fri, 30 Nov 2012 18:11:56 +0100
> From: Agustin Martin <agustin.martin@hispalinux.es>
> Cc: Akim Demaille <akim.demaille@gmail.com>
> 
> (if ispell-current-personal-dictionary ; Use specified pers dict.
>     (list "-p" (expand-file-name ispell-current-personal-dictionary)))
> 
> and is indeed affected by the changes in `default-dictionary' coming from
> `ispell-init-process`.
> 
> I think the only reason for that `expand-file-name' is the tilde expansion
> in non-UNIX systems. In UNIX systems tilde can be explicitly passed. 

You can only reliably pass the tilde to a shell, not to an arbitrary
program.

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Agustin Martin]

On Fri, Nov 30, 2012 at 01:49:14PM -0500, Stefan Monnier wrote:
> > I am now playing with something different, preserve original value of
> > `default-directory' in `current-ispell-directory' variable and expand
> > personal dictionary after it if no absolute path or tilde is used.
> 
> IIRC a related issue is whether the ispell process can be kept (rather
> than killed&restarted) when switching buffer.
> 
> For my use case, it's fairly important that the ispell process not be
> killed&restarted unnecessarily and indeed most of my buffers can share
> the same ispell process because they use no buffer-specific dictionary.

I committed a change regarding only with name expansion, but this has some
issues regarding ispell process keep/restart, so I am leaving this bug
report open until I also deal with this.

Former check in personal dictionaries was not in expanded name but in plain
string and this will not work well when files are in different directory. At
the same time I am noticing some related code that seems redundant, so this
will take a bit longer.

-- 
Agustin

bug#13019: 24.2; ispell: use of local ispell-personal-dictionary

[Agustin Martin]

On Mon, Dec 03, 2012 at 05:59:23PM +0100, Agustin Martin wrote:
> On Fri, Nov 30, 2012 at 01:49:14PM -0500, Stefan Monnier wrote:
> > > I am now playing with something different, preserve original value of
> > > `default-directory' in `current-ispell-directory' variable and expand
> > > personal dictionary after it if no absolute path or tilde is used.
> > 
> > IIRC a related issue is whether the ispell process can be kept (rather
> > than killed&restarted) when switching buffer.
> > 
> > For my use case, it's fairly important that the ispell process not be
> > killed&restarted unnecessarily and indeed most of my buffers can share
> > the same ispell process because they use no buffer-specific dictionary.
> 
> I committed a change regarding only with name expansion, but this has some
> issues regarding ispell process keep/restart, so I am leaving this bug
> report open until I also deal with this.
> 
> Former check in personal dictionaries was not in expanded name but in plain
> string and this will not work well when files are in different directory. At
> the same time I am noticing some related code that seems redundant, so this
> will take a bit longer.

Changes are apparently working very well, so is time for real life check. 

Committed. Closing bug report.

-- 
Agustin