embark icon indicating copy to clipboard operation
embark copied to clipboard

Kill-buffer action with embark-act-all for identically-named files

Open fredericgiquel opened this issue 1 year ago • 6 comments

If you try to close all buffers that visit identically-named files with emback-act-all, one buffer remains open.

Here's how to reproduce the problem (with minimal config):

  1. Launch Emacs with the following script:
#!/bin/sh

mkdir -p /tmp/emacs-embark
cd /tmp/emacs-embark
mkdir -p project1 project2 project3
touch project1/Makefile project2/Makefile project3/Makefile
cat  > .emacs << EOF
(package-initialize)
(unless (package-installed-p 'embark)
  (package-refresh-contents)
  (package-install 'embark))
EOF
HOME=$(pwd) emacs project1/Makefile project2/Makefile project3/Makefile
  1. M-x ibuffer and mark all Makefile buffers with m key (for each)
  2. M-x embark-act-all
  3. Choose k for kill-buffer and validate with y
  4. Refresh ibuffer with g and see that one Makefile buffer is still open

Step 2 can be replaced with operations in the minibuffer with the same result. In this case, the following error is printed: embark PCH: (error "No buffer named Makefile<project1>")

fredericgiquel avatar Jul 16 '23 08:07 fredericgiquel

I'll check when I'm at the computer, but I'm pretty sure I know what's going on. What embark-act-all does is just run the action in a loop, passing to it each of the candidates in turn. You must have Emacs configured so that those buffer names are deuniqified when there remains only one (I have Emacs configured that way and can't remember if it is the default or not). So the sequence of events is:

  1. You open the first Makefile, the buffer is named just Makefile.
  2. You open the second and third Makefile, the buffer are made unique by naming them Makefile<project1>, etc.
  3. You use embark-act-all to kill them:
    1. The first two are killed just fine, but know only one remains so it's renamed just Makefile.
    2. Now embark-act-all has only one buffer left to kill, named Makefile<project1> or something like that and cannot find it because it was renamed behind its back.

I'm not sure there is any easy way to improve this situation.

oantolin avatar Jul 16 '23 12:07 oantolin

One way to deal with this is to temporarily set uniquify-after-kill-buffer-p to nil when using kill-buffer as an action:

(cl-defun no-uniquify-after-kill-buffer (&rest args &key run &allow-other-keys)
  (let (uniquify-after-kill-buffer-p)
    (apply run args)))

(cl-pushnew #'no-uniquify-after-kill-buffer
            (alist-get 'kill-buffer embark-around-action-hooks))

This will avoid the error and let you kill the buffers in your example. Doing this has the disadvantage that if you kill all but one of the buffers, then the last remaining one does not get renamed.

I think there may not be a general solution to this problem I can add to Embark. But you can add that to your personal configuration if you prefer having its disadvantage than having the error you reported. I'll think a bit more about it, but maybe that's as good as it gets.

oantolin avatar Jul 16 '23 16:07 oantolin

Thanks for your answers.

I also dug into the subject on my own. There may be a more robust way to solve the problem.

kill-buffer accepts buffer name or buffer object as argument. The problem is that embark-act-all pass an old buffer name to kill-buffer (because of uniquify). But the buffer object is not modified by uniquify.

If we simulate step 2 to 4 (of the previous procedure) by the following piece of code:

(let ((buffer1 "Makefile<project1>")
      (buffer2 "Makefile<project2>")
      (buffer3 "Makefile<project3>"))
  (kill-buffer buffer1)
  (kill-buffer buffer2)
  (kill-buffer buffer3))

we facing the same problem related to uniquify (that changes the third buffer name).

But if we use:

(let ((buffer1 (get-buffer "Makefile<project1>"))
      (buffer2 (get-buffer "Makefile<project2>"))
      (buffer3 (get-buffer "Makefile<project3>")))
  (kill-buffer buffer1)
  (kill-buffer buffer2)
  (kill-buffer buffer3))

all 3 buffers are closed.

Is there any way to modify the candidates list before the loop to transform buffer names in buffer objects ?

fredericgiquel avatar Jul 16 '23 17:07 fredericgiquel

You can write your own multi-target actions that receives a list of buffer names, looks up the actual buffers and kills them:

(defun kill-each-buffer (buffer-names)
    "Kill each buffer whose name is in the list of BUFFER-NAMES."
  (mapc #'kill-buffer (mapcar #'get-buffer buffer-names)))

(add-to-list 'embark-multitarget-actions #'kill-each-buffer)

(setf (alist-get 'kill-each-buffer embark-post-action-hooks)
      '(embark--restart))

(keymap-set embark-buffer-map "k" #'kill-each-buffer)

That function can completely replace kill-buffer for Embark's purposes and solves this problem. I'm inclined to not add it to Embark however, since I feel this is a niche problem and don't want to lose the simplicity of reusing the standard kill-buffer command as an action. It's probably telling that this is only coming up now.

oantolin avatar Jul 16 '23 23:07 oantolin

Thank you for your help. I will try both solutions in my personal configuration.

Should I close the issue ?

fredericgiquel avatar Jul 18 '23 08:07 fredericgiquel

The second solution is better, you probably only need to try that one. About closing: let's keep it open for a bit while I think about whether to make a change in Embark or not.

oantolin avatar Jul 18 '23 17:07 oantolin