[emacs-w3m:11218] 24.0.50; don't ispell-kill-ispell over and over

[emacs-w3m:11218] 24.0.50; don't ispell-kill-ispell over and over

[jidanni]

Every time I even do a
  S runs the command w3m-search, which is an interactive compiled Lisp
  function in `w3m-search.el'.

I see a
  Starting new Ispell process [american] ...

Looking in *Messages*
  Note: file is write protected
  Ispell process killed
  Starting new Ispell process [american] ...
  Mark set
  Mark saved where search started [5 times]
  Ispell process killed
  Starting new Ispell process [american] ...
  Ispell process killed
  Making completion list...
  Scanning for dabbrevs...100%
  Starting new Ispell process [american] ...

Why couldn't things be left as they were? I.e., just let it live.
See http://jidanni.org/comp/configuration/ for my dotfiles if curious.

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Agustin Martin]

On Sun, May 09, 2010 at 09:21:45AM +0800, jidanni@jidanni.org wrote:
> Every time I even do a
>   S runs the command w3m-search, which is an interactive compiled Lisp
>   function in `w3m-search.el'.
> 
> I see a
>   Starting new Ispell process [american] ...
> 
> Looking in *Messages*
>   Note: file is write protected
>   Ispell process killed
>   Starting new Ispell process [american] ...
>   Mark set
>   Mark saved where search started [5 times]
>   Ispell process killed
>   Starting new Ispell process [american] ...
>   Ispell process killed
>   Making completion list...
>   Scanning for dabbrevs...100%
>   Starting new Ispell process [american] ...

The part of your dotfiles that triggers this is in .emacs-w3m

(add-hook
 'minibuffer-setup-hook
 (lambda ()
   (if(string-match "TEXT: \\| search: " (minibuffer-prompt))
       (flyspell-mode 1))))

and is related to my changes in bzr:#100056 killing ispell if buffer where
ispell process was generated is killed. (partially related to bzr:#100055)

The reason for that changes is that when emacs visits a file in a removable 
device and an ispell process is launched in that file, ispell process sets 
the dir in the removable device as default dir, so if one switch to other 
buffer outside the removable device and original buffer is killed, unless 
ispell process is restarted (personal dictionary, localwords, different 
language) or just killed, removable device stays bound and cannot be 
unmounted. 

This is the reason for ispell process being killed when buffer where it was 
created is killed. Since you create a buffer (the minibuffer) and a new
ispell process for it, this is the behavior you get. Note that this will
not happen if in the main buffer you already have an ispell process started 
for the same language you will use in the minibuffer.

Will think about it, but I do not see an easy workaround to take care of
both things reliably.

Cheers,

-- 
Agustin

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Stefan Monnier]

> The reason for that changes is that when emacs visits a file in a removable 
> device and an ispell process is launched in that file, ispell process sets 
> the dir in the removable device as default dir, so if one switch to other 
> buffer outside the removable device and original buffer is killed, unless 
> ispell process is restarted (personal dictionary, localwords, different 
> language) or just killed, removable device stays bound and cannot be 
> unmounted. 

Why not always start the ispell process in the / or $HOME directory?


        Stefan

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Agustin Martin]

On Tue, May 11, 2010 at 10:40:28AM -0400, Stefan Monnier wrote:
> > The reason for that changes is that when emacs visits a file in a removable 
> > device and an ispell process is launched in that file, ispell process sets 
> > the dir in the removable device as default dir, so if one switch to other 
> > buffer outside the removable device and original buffer is killed, unless 
> > ispell process is restarted (personal dictionary, localwords, different 
> > language) or just killed, removable device stays bound and cannot be 
> > unmounted. 
> 
> Why not always start the ispell process in the / or $HOME directory?

Note that ispell uses the directory where ispell is called as a 
directory that can contain personal dictionaries in parallel with 
the possible standard personal dictionary when no explicit personal
dictionary is set. Unconditionally changing to $HOME will break this
behavior.

Using / is not a different option in practice. Since it is not writable,
ispell-process-directory will automatically be changed to $HOME. Same 
for any other non-writable dir. This partially breaks the above for
non-writable dirs, but is standard ispell.el behavior for some time.

jidanni's report also made me note that if we ever implement buffer
based ispell processes (different process associated to every
buffer that request it) we will also face jidanni's problem (Thanks Dan).

I am still trying to think about something cleaner, or at least about a
reasonable workaround.

Cheers,

-- 
Agustin

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Agustin Martin]

On Tue, May 11, 2010 at 05:18:26PM +0200, Agustin Martin wrote:
> I am still trying to think about something cleaner, or at least about a
> reasonable workaround.

I have been thinking about this, and about a fix that is XEmacs compatible.
Although current ispell.el and flyspell.el cannot be used out of the box in
XEmacs, I am using for Debian one of the versions before the CVS->bzr
migration heavily patched to also work for XEmacs and keep a consistent
spellchecking interface across different Emacs flavors. As a matter of fact
it is even patched to work with emacs 21.3, but this is something I do not
want to maintain any longer.

I am thinking about two possible approaches, one is a compromise, minibuffer
is somewhat special, so ispell process directory could be set to $HOME only
when spellchecking minibuffer, and name of buffer where ispell process is
started faked in this specific case, so process is not killed when buffer is
killed. Since process is asigned to $HOME no problems are expected when this
is done from a buffer in a removable device creating a minibuffer. However,
usual ispell behavior will not be fully honoured here, but this is a
side use.

Something like

    (if (window-minibuffer-p)
	(setq ispell-process-directory (expand-file-name "~/")
	      ispell-process-buffer-name 
			" *Minibuffer-1-has-spellcheck-enabled")
      (setq ispell-process-directory default-directory
	    ispell-process-buffer-name (buffer-name)))

The second approach requires a clean and portable way of knowing the
parent buffer for the relevant minibuffer. In this case, just naming
ispell-process-buffer-name as parent buffer name may work. 

I currently do not see a good way to do the second, so first approach should
be a reasonable compromise. I will test a bit more and commit if no further
problems appear.

-- 
Agustin

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Lennart Borgman]

On Wed, May 12, 2010 at 12:26 PM, Agustin Martin
<agustin.martin@hispalinux.es> wrote:
>
> The second approach requires a clean and portable way of knowing the
> parent buffer for the relevant minibuffer.

Can minibuffer-selected-window be used?

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Agustin Martin]

On Wed, May 12, 2010 at 03:30:35PM +0200, Lennart Borgman wrote:
> On Wed, May 12, 2010 at 12:26 PM, Agustin Martin
> <agustin.martin@hispalinux.es> wrote:
> >
> > The second approach requires a clean and portable way of knowing the
> > parent buffer for the relevant minibuffer.
> 
> Can minibuffer-selected-window be used?

Yes, thanks, but not for all. For Emacs 22+ 
(window-buffer (minibuffer-selected-window)) should extract the 
needed info. 

Unfortunately this seems not available for XEmacs. I am afraid I will 
have to combine both if I want that to be portable, making FSF Emacs 
use the above and XEmacs fallback to the $HOME way.

Thanks for the feedback. Hope to check and commit tomorrow.

-- 
Agustin

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Stefan Monnier]

> I am thinking about two possible approaches, one is a compromise, minibuffer
> is somewhat special, so ispell process directory could be set to $HOME only
> when spellchecking minibuffer, and name of buffer where ispell process is

How about associating ispell processes with default-directories rather
than just with buffers?  I.e. share ispell processes between buffers
that share default-directory.  And use $HOME whenever possible
(i.e. when we can determine that there's no local dictionary in
default-directory).


        Stefan

bug#6143: 24.0.50; don't ispell-kill-ispell over and over

[Agustin Martin]

On Wed, May 12, 2010 at 10:28:26AM -0400, Stefan Monnier wrote:
> > I am thinking about two possible approaches, one is a compromise, minibuffer
> > is somewhat special, so ispell process directory could be set to $HOME only
> > when spellchecking minibuffer, and name of buffer where ispell process is
> 
> How about associating ispell processes with default-directories rather
> than just with buffers?  I.e. share ispell processes between buffers
> that share default-directory.  

That is current behavior unless buffers use different languages for
spellchecking or have a set of localwords defined (which may be different
for different buffers, needing process restart).

> And use $HOME whenever possible
> (i.e. when we can determine that there's no local dictionary in
> default-directory).

Doing this without the kill-ispell-on-kill-buffer machinery may still leave
some corner cases. 

Noticed that this dual personal dictionary behavior seems to be an ispell
only feature. If properly handling this is causing more harm than good, it
may even be dropped by forcing ispell-process-directory to always be $HOME.  

Anyway, I have commited a possible fix for jidanni's problem, making
parent-dir 'owner' of ispell process in a minibuffer'. Hope this fixes the
problem.

Closing bug report,

-- 
Agustin

bug#6143: closed (Re: bug#6143: 24.0.50; don't ispell-kill-ispell over and over)

[jidanni]

As of Debian emacs-snapshot package 1:20100524-1,
if one kills the buffer associated with the first ispell process, ispell
gets killed with it or something, when it should in fact better live on,
so that it need not be created again when a new (even mini-, for me) buffer is born.

bug#6143: ispell killing still not fixed

[jidanni]

reopen 6143
thanks
I'm sorry but this is still not fixed for me as of
emacs-snapshot 1:20100619-2

I would be happy if you told me how to workaround it in
my http://jidanni.org/comp/configuration/.emacs .

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Sun, Jun 20, 2010 at 12:47:40PM +0800, jidanni@jidanni.org wrote:
> reopen 6143
> thanks
> I'm sorry but this is still not fixed for me as of
> emacs-snapshot 1:20100619-2

Seems fixed here. It will only happen if main buffer defines an
`ispell-local-dictionary' different than default `ispell-local-dictionary',
where there is no way to know which language you prefer for spellchecking
when in the minibuffer. And process will be restarted if dictionaries are
different or there is a change in localwords/not localwords presence.

> I would be happy if you told me how to workaround it in
> my http://jidanni.org/comp/configuration/.emacs .

> (setq ispell-dictionary "american")

You should use (setq ispell-local-dictionary "american") in your .emacs
file to set global dictionary default.

Not sure if this what is triggering the problem at your side. If the problem
persists, please attach a minimal file where that problem happens.

Cheers,

-- 
Agustin

bug#6143: ispell killing still not fixed

[jidanni]

AM> You should use (setq ispell-local-dictionary "american") in your .emacs
Done, as you now see in
http://jidanni.org/comp/configuration/.emacs
However filling form fields in emacs-w3m triggers one-shot ispells still.
Isn't there a way to just have one permanent ispell that lives as long
as the emacs session?

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Wed, Jun 23, 2010 at 10:19:12AM +0800, jidanni@jidanni.org wrote:
> AM> You should use (setq ispell-local-dictionary "american") in your .emacs
> Done, as you now see in
> http://jidanni.org/comp/configuration/.emacs
> However filling form fields in emacs-w3m triggers one-shot ispells still.

Has this fixed the over and over ispell process restart? If so, that one
shot ispell is the expected behavior. ispell process is started first time
spellchecking is requested.

> Isn't there a way to just have one permanent ispell that lives as long
> as the emacs session?

There is no reason to open an ispell process at emacs startup unless you
really want to do spellchecking. If what you ask is if is possible to
unconditionally start an ispell process from your .emacs to avoid it to 
be restarted later unless really needed, I have always got a single extra
restart. Will try to investigate this a bit more.

Cheers,

-- 
Agustin

bug#6143: ispell killing still not fixed

[jidanni]

AM> Has this fixed the over and over ispell process restart?

No. Anyway, as it is hard to detect just what is causing the problem,
can we just have a flag, e.g., (setq ispell-never-kill 't) so that once
ispell is started, it is never killed until emacs quits. And thus will
be detected too, so no second ispell will be started.

bug#6143: ispell killing still not fixed

[Agustin Martin]

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

On Thu, Jun 24, 2010 at 10:43:09PM +0800, jidanni@jidanni.org wrote:
> AM> Has this fixed the over and over ispell process restart?
> 
> No. Anyway, as it is hard to detect just what is causing the problem,
> can we just have a flag, e.g., (setq ispell-never-kill 't) so that once
> ispell is started, it is never killed until emacs quits. And thus will
> be detected too, so no second ispell will be started.

I tend to disagree here (but have good news),

ispell process should not be restarted unless needed. And if needed should
be restarted. Other behavior may make more harm than good and hide
underlying problems.

The good news are that seems I found the underlying problem (at least for
the single extra ispell restart). Directory value, being the same, was 
sometimes set with unexpanded `~' and sometimes with it expanded, so they 
looked different to the comparison operator. 

I am testing attached patch that should make the expanded version to be
used everywhere. I am testing it along with a dirty hack in .emacs

(setq ispell-local-dictionary "american")
(load "ispell")
(setq ispell-program-name "aspell")
(ispell-set-spellchecker-params)
(ispell-start-process)

that should start an ispell process. With the attached changes at least
extra restart seems to be fixed.

If I see no further problems I will commit changes soon, so you can test
them in your box.

Thanks for your feedback,

-- 
Agustin

[-- Attachment #2: ispell.el_expand-default-directory.diff --]
[-- Type: text/x-diff, Size: 1470 bytes --]

--- ispell.el.orig	2010-06-24 16:57:52.000000000 +0200
+++ ispell.el	2010-06-24 17:02:22.000000000 +0200
@@ -2622,7 +2622,7 @@
 	   ;; Restart check for personal dictionary is done in
 	   ;; `ispell-internal-change-dictionary', called from `ispell-buffer-local-dict'
 	   (or (or ispell-local-pdict ispell-personal-dictionary)
-	       (equal ispell-process-directory default-directory)))
+	       (equal ispell-process-directory (expand-file-name default-directory))))
       (setq ispell-filter nil ispell-filter-continue nil)
     ;; may need to restart to select new personal dictionary.
     (ispell-kill-ispell t)
@@ -2638,13 +2638,13 @@
     (if (window-minibuffer-p)
 	(if (fboundp 'minibuffer-selected-window)
 	    ;; Assign ispell process to parent buffer
-	    (setq ispell-process-directory default-directory
+	    (setq ispell-process-directory (expand-file-name default-directory)
 		  ispell-process-buffer-name (window-buffer (minibuffer-selected-window)))
 	  ;; Force `ispell-process-directory' to $HOME and use a dummy name
 	  (setq ispell-process-directory (expand-file-name "~/")
 		ispell-process-buffer-name " * Minibuffer-has-spellcheck-enabled"))
       ;; Not in a minibuffer
-      (setq ispell-process-directory default-directory
+      (setq ispell-process-directory (expand-file-name default-directory)
 	    ispell-process-buffer-name (buffer-name)))
     (if ispell-async-processp
 	(set-process-filter ispell-process 'ispell-filter))

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Thu, Jun 24, 2010 at 05:23:45PM +0200, Agustin Martin wrote:
> I am testing attached patch that should make the expanded version to be
> used everywhere. I am testing it along with a dirty hack in .emacs
> 
> (setq ispell-local-dictionary "american")
> (load "ispell")
> (setq ispell-program-name "aspell")
> (ispell-set-spellchecker-params)
> (ispell-start-process)
> 
> that should start an ispell process. With the attached changes at least
> extra restart seems to be fixed.

But a further ispell process (triggered from e.g. flyspell) seems to not 
use process created by above code but spawn a new one. So, please discard
code above, it does not properly register ispell process. 

patch included in my previous mail is OK, and should avoid some restarts.
Still testing ...

-- 
Agustin

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Thu, Jun 24, 2010 at 05:51:20PM +0200, Agustin Martin wrote:
> On Thu, Jun 24, 2010 at 05:23:45PM +0200, Agustin Martin wrote:
> > I am testing attached patch that should make the expanded version to be
> > used everywhere. I am testing it along with a dirty hack in .emacs
> > 
> > (setq ispell-local-dictionary "american")
> > (load "ispell")
> > (setq ispell-program-name "aspell")
> > (ispell-set-spellchecker-params)
> > (ispell-start-process)
> > 
> > that should start an ispell process. With the attached changes at least
> > extra restart seems to be fixed.
> 
> But a further ispell process (triggered from e.g. flyspell) seems to not 
> use process created by above code but spawn a new one. So, please discard
> code above, it does not properly register ispell process. 

Seems I used the wrong function, something like

(setq ispell-local-dictionary "american")
(setq ispell-program-name "aspell")
(load "ispell")
(ispell-set-spellchecker-params)
(ispell-init-process)

in ~/.emacs seems to work better (together with my patch). A bit ugly, but
that may give you the behavior you want.

-- 
Agustin

bug#6143: ispell killing still not fixed

[jidanni]

What I will do is just wait for the next version of
emacs-snapshot:
 *** 1:20100619-2 0
        500 http://emacs.orebokech.com sid/main Packages
which should contain your fixes. That way I won't need to modify my
.emacs file I suppose.

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Fri, Jun 25, 2010 at 07:52:17AM +0800, jidanni@jidanni.org wrote:
> What I will do is just wait for the next version of
> emacs-snapshot:
>  *** 1:20100619-2 0
>         500 http://emacs.orebokech.com sid/main Packages
> which should contain your fixes. That way I won't need to modify my
> .emacs file I suppose.

Changes committed (bzr#100641). Not yet closing bug report.

Thanks for your feedback,

-- 
Agustin

bug#6143: totally not fixed

[agustin.martin]

Sorry, this is totally not fixed as of emacs-snapshot 1:20100705-1.

Please send me a workaround that I can put in my .emacs file.
It is driving me nuts.

Just start the process once for me please.

bug#6143: 6143 ispell not fixed

[jidanni]

Sorry, this is totally not fixed as of
Package: emacs-snapshot
Version: 1:20100711-1

Is there any workaround? Thanks.

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Fri, Jul 23, 2010 at 03:11:19AM +0800, jidanni@jidanni.org wrote:
> Sorry, this is totally not fixed as of
> Package: emacs-snapshot
> Version: 1:20100711-1
> 
> Is there any workaround? Thanks.

I am confused, this is working perfectly for me. Tried with the scratch
buffer and a reduced .emacs file containing only

(setq ispell-local-dictionary "american")
(setq ispell-program-name "aspell")

(load "ispell")
(ispell-set-spellchecker-params)
(ispell-init-process)


(add-hook
  'minibuffer-setup-hook
  (lambda ()
    (if(string-match "TEXT: \\| search: " (minibuffer-prompt))
        (flyspell-mode 1))))

Does yout file contain something special?

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Dan Nicolaescu]

You can get the "Ispell process killed" message by doing:

$ echo $LANG
C
$ emacs -Q
M-: (add-hook 'text-mode-hook       'flyspell-mode) RET

C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
type something
C-x v v
type something in the log-edit buffer.
C-c C-c

now look at the *Messages* buffer and see the "Ispell process killed"

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
> 
> You can get the "Ispell process killed" message by doing:
> 
> $ echo $LANG
> C
> $ emacs -Q
> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> 
> C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
> type something
> C-x v v
> type something in the log-edit buffer.
> C-c C-c
> 
> now look at the *Messages* buffer and see the "Ispell process killed"

Strange, I get a different result (although also with an error),

$ LANG=C LC_ALL=C emacs-snapshot -Q &
M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
C-x C-f A_FILE_UNDER_GIT_VERSION_CONTROL (kkk.txt)
type something
C-x v v
type something in the log-edit buffer.

  Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))

C-c C-c

  Buffer kkk.txt modified; save it? (y or n) 
  Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))

But no ispell process restart.

Tested with Debian GNU/Linux emacs-snapshot 20100711 which should contain
all recent *spell changes.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Mon, Jul 26, 2010 at 11:38:49AM +0200, Agustin Martin wrote:
> On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
> > 
> > You can get the "Ispell process killed" message by doing:
> > 
> > $ echo $LANG
> > C
> > $ emacs -Q
> > M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> > 
> > C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
> > type something
> > C-x v v
> > type something in the log-edit buffer.
> > C-c C-c
> > 
> > now look at the *Messages* buffer and see the "Ispell process killed"
> 
> Strange, I get a different result (although also with an error),

And more funny, I can reproduce your original problem with last version of
dictionaries-common I uploaded to Debian, which contains bleedeng edge
*spell.el from FSF Emacs bzr heavily patched to work with XEmacs and some
older Emacs flavours (At some time I will try to push the XEmacs stuff into
the bzr repo) with no special precautions (In my .emacs file I instruct to
enable flyspell mode for text files),

$ emacs kkk.txt &
C-x v v

And process is restarted. This seems to happen only once and I could only
reproduce this in this special case. Will try to investigate.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Mon, Jul 26, 2010 at 11:59:07AM +0200, Agustin Martin wrote:
> On Mon, Jul 26, 2010 at 11:38:49AM +0200, Agustin Martin wrote:
> > On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
> > > 
> > > You can get the "Ispell process killed" message by doing:
> > > 
> > > $ echo $LANG
> > > C
> > > $ emacs -Q
> > > M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> > > 
> > > C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
> > > type something
> > > C-x v v
> > > type something in the log-edit buffer.
> > > C-c C-c
> > > 
> > > now look at the *Messages* buffer and see the "Ispell process killed"
> > 
> > Strange, I get a different result (although also with an error),
> 
> And more funny, I can reproduce your original problem with last version of
> dictionaries-common I uploaded to Debian, which contains bleedeng edge
> *spell.el from FSF Emacs bzr heavily patched to work with XEmacs and some
> older Emacs flavours (At some time I will try to push the XEmacs stuff into
> the bzr repo) with no special precautions (In my .emacs file I instruct to
> enable flyspell mode for text files),

Forgot to mention that along with Emacs 23.2, not with bleeding edge Emacs.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Mon, Jul 26, 2010 at 11:59:07AM +0200, Agustin Martin wrote:
> On Mon, Jul 26, 2010 at 11:38:49AM +0200, Agustin Martin wrote:
> > On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
> > > 
> > > You can get the "Ispell process killed" message by doing:
> > > 
> > > $ echo $LANG
> > > C
> > > $ emacs -Q
> > > M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> > > 
> > > C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
> > > type something
> > > C-x v v
> > > type something in the log-edit buffer.
> > > C-c C-c
> > > 
> > > now look at the *Messages* buffer and see the "Ispell process killed"
> > 
> > Strange, I get a different result (although also with an error),
> 
> And more funny, I can reproduce your original problem with last version of
> dictionaries-common I uploaded to Debian, which contains bleedeng edge
> *spell.el from FSF Emacs bzr heavily patched to work with XEmacs and some
> older Emacs flavours (At some time I will try to push the XEmacs stuff into
> the bzr repo) with no special precautions (In my .emacs file I instruct to
> enable flyspell mode for text files),
> 
> $ emacs kkk.txt &
> C-x v v
> 
> And process is restarted. This seems to happen only once and I could only
> reproduce this in this special case. Will try to investigate.

An even more fun (and desperation), I cannot reproduce this today with the
same file and the same emacs23.2 version + ispell.el from Debian 
dictionaries-common, when I was trying to investigate what was triggering 
that behavior.

Will try to keep an eye on this,

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Dan Nicolaescu]

Agustin Martin <agustin.martin@hispalinux.es> writes:

> On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
>> 
>> You can get the "Ispell process killed" message by doing:
>> 
>> $ echo $LANG
>> C
>> $ emacs -Q
>> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
>> 
>> C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
>> type something
>> C-x v v
>> type something in the log-edit buffer.
>> C-c C-c
>> 
>> now look at the *Messages* buffer and see the "Ispell process killed"
>
> Strange, I get a different result (although also with an error),
>
> $ LANG=C LC_ALL=C emacs-snapshot -Q &
> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> C-x C-f A_FILE_UNDER_GIT_VERSION_CONTROL (kkk.txt)
> type something
> C-x v v
> type something in the log-edit buffer.
>
>   Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))
>
> C-c C-c
>
>   Buffer kkk.txt modified; save it? (y or n) 

Can you try to first edit the kkk.txt file, save it, and then invoke C-x v v ? 
(No idea if that makes a difference, but...)

bug#6143: 6143 ispell not fixed

[Dan Nicolaescu]

Agustin Martin <agustin.martin@hispalinux.es> writes:

> On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
>> 
>> You can get the "Ispell process killed" message by doing:
>> 
>> $ echo $LANG
>> C
>> $ emacs -Q
>> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
>> 
>> C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
>> type something
>> C-x v v
>> type something in the log-edit buffer.
>> C-c C-c
>> 
>> now look at the *Messages* buffer and see the "Ispell process killed"
>
> Strange, I get a different result (although also with an error),
>
> $ LANG=C LC_ALL=C emacs-snapshot -Q &
> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
> C-x C-f A_FILE_UNDER_GIT_VERSION_CONTROL (kkk.txt)
> type something
> C-x v v
> type something in the log-edit buffer.
>
>   Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))
>
> C-c C-c
>
>   Buffer kkk.txt modified; save it? (y or n) 
>   Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))
>
> But no ispell process restart.

When I ispell-kill-ispell is invoked the backtrace looks like this:

 ispell-kill-ispell(t)
  (if (equal ispell-process-buffer-name (buffer-name)) (ispell-kill-ispell t))
  (lambda nil (if (equal ispell-process-buffer-name ...) (ispell-kill-ispell t)))()
  kill-buffer(#<buffer *VC-log*>)
  vc-finish-logentry()
  call-interactively(vc-finish-logentry)
  log-edit-done()
  call-interactively(log-edit-done nil nil)


So this is caused by:

(add-hook 'kill-buffer-hook
            '(lambda ()
                 (if (equal ispell-process-buffer-name (buffer-name))
                      (ispell-kill-ispell t))))


ispell-process-buffer-name is "*VC-log*"

bug#6143: 6143 ispell not fixed

[Agustin Martin]

2010/7/27 Dan Nicolaescu <dann@gnu.org>:
> Agustin Martin <agustin.martin@hispalinux.es> writes:
>
>> On Sat, Jul 24, 2010 at 01:35:06AM -0400, Dan Nicolaescu wrote:
>>>
>>> You can get the "Ispell process killed" message by doing:
>>>
>>> $ echo $LANG
>>> C
>>> $ emacs -Q
>>> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
>>>
>>> C-x C-f A_FILE_UNDER_VERSION_CONTROL_FOR_EXAMPLE_MANAGED_BY_GIT
>>> type something
>>> C-x v v
>>> type something in the log-edit buffer.
>>> C-c C-c
>>>
>>> now look at the *Messages* buffer and see the "Ispell process killed"
>>
>> Strange, I get a different result (although also with an error),
>>
>> $ LANG=C LC_ALL=C emacs-snapshot -Q &
>> M-: (add-hook 'text-mode-hook       'flyspell-mode) RET
>> C-x C-f A_FILE_UNDER_GIT_VERSION_CONTROL (kkk.txt)
>> type something
>> C-x v v
>> type something in the log-edit buffer.
>>
>>   Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))
>>
>> C-c C-c
>>
>>   Buffer kkk.txt modified; save it? (y or n)
>>   Error during redisplay: (error No match 4 in highlight (4 font-lock-warning-face))
>>
>> But no ispell process restart.
>
> When I ispell-kill-ispell is invoked the backtrace looks like this:
>
>  ispell-kill-ispell(t)
>  (if (equal ispell-process-buffer-name (buffer-name)) (ispell-kill-ispell t))
>  (lambda nil (if (equal ispell-process-buffer-name ...) (ispell-kill-ispell t)))()
>  kill-buffer(#<buffer *VC-log*>)
>  vc-finish-logentry()
>  call-interactively(vc-finish-logentry)
>  log-edit-done()
>  call-interactively(log-edit-done nil nil)
>
>
> So this is caused by:
>
> (add-hook 'kill-buffer-hook
>            '(lambda ()
>                 (if (equal ispell-process-buffer-name (buffer-name))
>                      (ispell-kill-ispell t))))
>
>
> ispell-process-buffer-name is "*VC-log*"

Thanks for debugging, Dan

I guess your original A_FILE_UNDER_GIT_VERSION_CONTROL file is not a
text-mode file. If so, this is the currently expected behavior, do not
leave unused ispell processes behind.  So, no ispell process is
started for initial file, and only when you start the text mode buffer
"*VC-log*", an ispell process is started. Since there was no previous
process "owned" by a previous buffer, this is killed on buffer kill.

Note that I was playing with a text file, so an ispell process is
started for it and, since it does not contain neither localwords nor
an explicit language different from default, same process is used for
"*VC-log*" buffer and is not killed on "*VC-log*" kill since it was
initiated from original buffer. That is the difference I find. What
happened before for me is that I probably did not use the file as
kkk.txt, but as plain kkk.

This being too noisy or not is open for discussion, others may argue
that leaving unused ispell processes behind is also a bug.  I
personally do not find this noisy enough. Opinions welcome.

If this is considered too noisy and leaving unused ispell processes
behind not a problem I think the way to go is to always use "~/" as
ispell process directory, so problem with removable media that used
the kill-on-kill gets also fixed. Better if there is a not too
complicated way of having an exception when Ispell is the
spellchecking engine and original directory contains an Ispell
directory dictionary for given language. This way if using Ispell,
current directory is used as ispell-default-directory only if contains
appropriate directory personal dictionary and process is killed on
buffer kill only if so (IIRC someone already proposed this, but I do
not find original mail).  Otherwise we lose support for this Ispell
funcionality. Have to think a bit about this.

PS: I have been lately with limited time and connectivity. Do not
expect something quickly.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[Stefan Monnier]

> If this is considered too noisy and leaving unused ispell processes
> behind not a problem I think the way to go is to always use "~/" as
> ispell process directory, so problem with removable media that used
> the kill-on-kill gets also fixed. Better if there is a not too

That's the way I would prefer: leaving an ispell process running is not
a big deal, especially if it comes with some kind of timeout (so it
does get killed if it's unused for more than N minutes).

Regarding support for directory-specific dictionaries, I have no strong
opinion on this, except that I'd rather not have to pay for it in the
case where I don't use it (as is my case, and I expect many people's
case as well).


        Stefan

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Thu, Jul 29, 2010 at 03:07:35PM +0200, Stefan Monnier wrote:
> > If this is considered too noisy and leaving unused ispell processes
> > behind not a problem I think the way to go is to always use "~/" as
> > ispell process directory, so problem with removable media that used
> > the kill-on-kill gets also fixed. Better if there is a not too
> 
> That's the way I would prefer: leaving an ispell process running is not
> a big deal, especially if it comes with some kind of timeout (so it
> does get killed if it's unused for more than N minutes).
> 
> Regarding support for directory-specific dictionaries, I have no strong
> opinion on this, except that I'd rather not have to pay for it in the
> case where I don't use it (as is my case, and I expect many people's
> case as well).

I have just commited a change that should use "~/" as default-directory
unless Ispell per-directory personal dicts are used and not in a minibuffer
under XEmacs. kill-on-exit is no longer done if ispell-process directory is
"~/", so I expect this to achieve the best of both worlds.

diff looks large because of re-indenting (a new let* is added), but changes
are smaller.

Hope this is a better approach to the problem. Will wait for further testing
(cross fingers) before closing the bug report.

Thanks all for the feedback.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[jidanni]

Thanks. I'll get your change in the next Debian emacs-snapshot.

bug#6143: ispell killing still not fixed

[Agustin Martin]

On Thu, Sep 02, 2010 at 09:03:57PM +0800, jidanni@jidanni.org wrote:
> On Thu, Sep 02, 2010 at 02:47:47PM +0200, Agustin Martin wrote:
> > I have just commited a change that should use "~/" as default-directory
> > unless Ispell per-directory personal dicts are used and not in a
> > minibuffer
> >  under XEmacs. kill-on-exit is no longer done if ispell-process
> > directory is
> > "~/", so I expect this to achieve the best of both worlds.
> Thanks. I'll get your change in the next Debian emacs-snapshot.

Hi, Dan,

Could you please check if this bug report is still valid, so we can 
close it if fixed.

On Mon, Jun 21, 2010 at 02:10:04PM +0200, Agustin Martin wrote:
> On Sun, Jun 20, 2010 at 12:47:40PM +0800, jidanni@jidanni.org wrote:
> > I would be happy if you told me how to workaround it in
> > my http://jidanni.org/comp/configuration/.emacs .
> 
> > (setq ispell-dictionary "american")
> 
> You should use (setq ispell-local-dictionary "american") in your .emacs
> file to set global dictionary default.

By the way, I was wrong about this, is ispell-dictionary what should   
be used for global dictionary default, as in your original config file.
Sorry for the noise.

Thanks for your feedback,
   
-- 
Agustin

bug#6143: 6143 ispell not fixed

[Stefan Monnier]

> I have just commited a change that should use "~/" as default-directory
> unless Ispell per-directory personal dicts are used and not in a minibuffer
> under XEmacs.

Could you explain (ideally in a comment in the code) why the minibuffer
case is important, why we distinguish the Emacs-vs-XEmacs cases, and why
we use (fboundp 'minibuffer-selected-window) rather than (featurep
'xemacs) to check whether we're in XEmacs (I see this use comes
from the earlier code, but maybe it's an opportunity to change it,
since (featurep 'xemacs) is treated specially by the byte-compiler to
generate more efficient code and to avoid spurious warnings).


        Stefan

bug#6143: 6143 ispell not fixed

[Agustin Martin]

On Thu, Sep 02, 2010 at 06:01:10PM +0200, Stefan Monnier wrote:
> > I have just commited a change that should use "~/" as default-directory
> > unless Ispell per-directory personal dicts are used and not in a minibuffer
> > under XEmacs.
> 
> Could you explain (ideally in a comment in the code) why the minibuffer
> case is important, 

Minibuffer case is now a very marginal case which will currently appear only
when directory-specific dictionaries are used under Ispell, and someone tries 
to spell-check stuff in the minibuffer (like jidanni did for FSF Emacs).
Without the change, in that very marginal case, everytime the minibuffer is 
closed ispell process will be killed.

> why we distinguish the Emacs-vs-XEmacs cases, 

I try to introduce as few XEmacs incompatibilities as possible, even if
current ispell.el and flyspell.el do not work under XEmacs. I still use FSF
Emacs ispell.el and flyspell.el as base for Debian, patched so they work even
with XEmacs.

I'd ideally like to decrease the number of Debian changes added just for
XEmacs compatibility by committing some of them to bzr repo. 

Also, since most of the Debian changes are already committed to FSF Emacs 
bzr repo, I'd like to sit down and see what else is needed for FSF Emacs 
ispell.el and flyspell.el to be integrated with our Debian dictionary
registration mechanism. If that seems of enough wide use, I'd like to push
changes to bzr.

I have some other changes pushed to Debian that should go into FSF Emacs
repo (I sometimes try things first in Debian).

> and why
> we use (fboundp 'minibuffer-selected-window) rather than (featurep
> 'xemacs) to check whether we're in XEmacs (I see this use comes
> from the earlier code, but maybe it's an opportunity to change it,
> since (featurep 'xemacs) is treated specially by the byte-compiler to
> generate more efficient code and to avoid spurious warnings).

Someone asked Stephen Turnbull if it was posible to implement
`minibuffer-selected-window' in XEmacs and he asked for code. So, I left
that possibility open.

Unless is a known design choice, I usually prefer to check for
variable/function availability, in case that is later implemented.

-- 
Agustin

bug#6143: 6143 ispell not fixed

[jidanni]

I will write back when I can reproduce it with you small file.

bug#6143: 6143 ispell not fixed

[jidanni]

Agustin, what do you say about Dan Nicolaescu's finding?

bug#6143: 6143 ispell not fixed

[jidanni]

So there are so many different outside factors affecting this bug.
Therefore you probably need to make a new variable ispell-never-kill, to
keep ispell alive, no matter what the outside conditions. Ugly, but needed.

bug#6143: 6143 ispell not fixed

[jidanni]

>>>>> "AM" == Agustin Martin <agustin.martin@hispalinux.es> writes:
AM> An even more fun (and desperation), I cannot reproduce this today with the
AM> same file and the same emacs
See, too many factors. So instead just make a variable that users can
turn on: ispell-never-die-once-started, to make it invincible.

bug#6143: 6143 ispell not fixed

[jidanni]

Yes it is no big deal for me if an ispell process never dies in emacs
once it is started.

And I never use .ispell or whatever files per directory.

Just stop this awful starting/stopping I see all day that appeared this
summer, please.

bug#6143: 6143 ispell not fixed

[jidanni]

Austin, is your email not bouncing?
Austin, can you fix the bug?

bug#6143: ispell killing still not fixed

[jidanni]

AM> Could you please check if this bug report is still valid, so we can
AM> close it if fixed.

Lately it hasn't bothered me. Must be fixed. Closing.

AM> By the way, I was wrong about this, is ispell-dictionary what should
AM> be used for global dictionary default, as in your original config
AM> file.

OK. I'll change it back. Thanks.