problem with uniquify.el and kill-buffer-hook

problem with uniquify.el and kill-buffer-hook

[Miles Bader]

Recently I've noticed that the `uniquify' package has stopped working
correctly, in particular, the feature controlled by the
`uniquify-after-kill-buffer-p' variable doesn't work; here's the doc:

   uniquify-after-kill-buffer-p's value is t

   If non-nil, rerationalize buffer names after a buffer has been killed.
   This can be dangerous if Emacs Lisp code is keeping track of buffers
   by their names (rather than keeping pointers to the buffers
   themselves).

I looked at the code a bit, and I think I found the problem:

This feature works by adding an appropriate local hook to
`kill-buffer-hook' in all the affected buffers.

However, this is done _before_ the buffer's mode is set (in the new
buffer that caused uniqify to be triggered); since most mode functions
call `kill-all-local-variables', this trashes the `kill-buffer-hook'.

I've found that I can fix the problem by doing:

  (put 'kill-buffer-hook 'permanent-local t)

But doing this by default may cause problems for other code (e.g., if a
particular mode adds a `kill-buffer-hook', you probably want that hook
to be deleted if the mode changes).

Since `kill-all-local-variables' first calls `change-major-mode-hook' I
guess another solution would be to make uniquify add some hair to
`change-major-mode-hook' that tries to preserve its entry in
`kill-buffer-hook' across the kill-all-local-variables', but I'm not
sure how this can be done without another hook that runs _after_
kill-all-local-variables has done its job.

Does anyone have any ideas for a solution?  The real problem here seems
to be that kill-buffer-hook can contain both mode-specific and
non-mode-specific entries; I'm not sure if any modes actually use
kill-buffer-hook though...

Also, any ideas why this just stopped working recently?  I didn't see
any _obvious_ things in the ChangeLogs, but maybe I just missed it.

Another thing (whew!), is that uniquify.el uses `defadvice' (on
`rename-buffer', and `create-file-buffer') and I wonder if it should be
changed to just have those functions invoke it directly.

Anyway sorry for the long rambling message!

Thanks,

-Miles
-- 
Ich bin ein Virus. Mach' mit und kopiere mich in Deine .signature.

Re: problem with uniquify.el and kill-buffer-hook

[Stefan Monnier]

> Recently I've noticed that the `uniquify' package has stopped working
> correctly, in particular, the feature controlled by the
> `uniquify-after-kill-buffer-p' variable doesn't work; here's the doc:
> 
>    uniquify-after-kill-buffer-p's value is t
> 
>    If non-nil, rerationalize buffer names after a buffer has been killed.
>    This can be dangerous if Emacs Lisp code is keeping track of buffers
>    by their names (rather than keeping pointers to the buffers
>    themselves).
> 
> I looked at the code a bit, and I think I found the problem:
> 
> This feature works by adding an appropriate local hook to
> `kill-buffer-hook' in all the affected buffers.
> 
> However, this is done _before_ the buffer's mode is set (in the new
> buffer that caused uniqify to be triggered); since most mode functions
> call `kill-all-local-variables', this trashes the `kill-buffer-hook'.

Duh!

> Also, any ideas why this just stopped working recently?  I didn't see
> any _obvious_ things in the ChangeLogs, but maybe I just missed it.

It's due to:

2002-09-27  Stefan Monnier  <monnier@cs.yale.edu>

	* uniquify.el (uniquify-rationalize-file-buffer-names):
	Add to kill-buffer-hook for buffers with conflict.
	(rename-buffer): Check kill-buffer-hook to see if there was a conflict.
	(uniquify-delay-rationalize-file-buffer-names): Remove useless check.
	(kill-buffer-hook): Don't change globally.

I guess it should just be reverted.  Please do it (unless you can come
up with a fix, of course).  I don't have easy access to the CVS right now.

This patch did not fix any real bug anyway.

> Another thing (whew!), is that uniquify.el uses `defadvice' (on
> `rename-buffer', and `create-file-buffer') and I wonder if it should be
> changed to just have those functions invoke it directly.

That's a long standing issue indeed.


	Stefan

Re: problem with uniquify.el and kill-buffer-hook

[Richard Stallman]

    I've found that I can fix the problem by doing:

      (put 'kill-buffer-hook 'permanent-local t)

    But doing this by default may cause problems for other code

I grepped for kill-buffer-hook and looked at some of the uses.
It looks like many others would be helped by this change
and none would be hurt by it.

Would someone like to check all of them?  The total number is only 60
or so.

Re: problem with uniquify.el and kill-buffer-hook

[Miles Bader]

On Sat, Dec 14, 2002 at 01:31:51PM -0500, Richard Stallman wrote:
>       (put 'kill-buffer-hook 'permanent-local t)
>     But doing this by default may cause problems for other code
> 
> I grepped for kill-buffer-hook and looked at some of the uses.
> It looks like many others would be helped by this change
> and none would be hurt by it.

Hmmm, it didn't occur to me that that might be the case!

If kill-buffer-hook is made permanent-local, should the documentation mention
that this is the case?  I looked at some other permanent-local variables, and
none seem to say so in their doc-string.

[Incidentally, some of them seem rather suspicious; for instance, why is
`enriched-mode' (the mode variable for enriched-mode) permanent-local?!
Is enriched-mode not a normal major mode?]

> Would someone like to check all of them?  The total number is only 60
> or so.

If no one else gets around to it, I'll do this when I get some time.

-Miles
-- 
o The existentialist, not having a pillow, goes everywhere with the book by
  Sullivan, _I am going to spit on your graves_.

Re: problem with uniquify.el and kill-buffer-hook

[Richard Stallman]

    [Incidentally, some of them seem rather suspicious; for instance, why is
    `enriched-mode' (the mode variable for enriched-mode) permanent-local?!
    Is enriched-mode not a normal major mode?]

It is not a major mode at all.

    If kill-buffer-hook is made permanent-local, should the
    documentation mention that this is the case?

I think it should.

Re: problem with uniquify.el and kill-buffer-hook

[Stefan Monnier]

>     If kill-buffer-hook is made permanent-local, should the
>     documentation mention that this is the case?
> I think it should.

Agreed.
It shouldn't be in the docstring, but it should be added by C-h v
instead (just as was done for buffer-localness).


	Stefan

Re: problem with uniquify.el and kill-buffer-hook

[Richard Stallman]

    It shouldn't be in the docstring, but it should be added by C-h v
    instead (just as was done for buffer-localness).

That is a good idea.