Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi-occur is not very comfortable with selectrum #226

Closed
minad opened this issue Nov 6, 2020 · 9 comments
Closed

multi-occur is not very comfortable with selectrum #226

minad opened this issue Nov 6, 2020 · 9 comments

Comments

@minad
Copy link
Contributor

minad commented Nov 6, 2020

If I am using selectrum-mode and call M-x multi-occur it asks me to select multiple buffers and finish with "RET". Since selectrum already preselects the first choice, I have to move the cursor up to finish, which is not very comfortable. Probably selectrum should not preselect in that case? Furthermore after selecting one buffer it should not appear again in the list of choices.

I am not sure if something like this could be fixed in selectrum, since it is a special case. Maybe some custom wrapper could be written around multi-occur? Maybe someone had the same issue and there is already a good solution for the problem?

@clemera
Copy link
Collaborator

clemera commented Nov 7, 2020

Thanks for your input! I agree that the problem is more with the command itself than with selectrum. This is a great candidate for completing-read-multiple (which is automatically enhanced by selectrum-mode):

(defun my-multi-occur ()
  (interactive)
  (let ((bufs
         (mapcar #'get-buffer
                 (completing-read-multiple "Buffer: "
                                           #'internal-complete-buffer))))
    (apply #'multi-occur (cons bufs (occur-read-primary-args)))))

To exit with a single buffer you exit with RET, to select multiple add more with TAB until you want to exit with RET. If you pressed TAB to often just delete the final comma and afterwards exit with RET.

@clemera clemera added the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Nov 7, 2020
@minad
Copy link
Contributor Author

minad commented Nov 7, 2020

@clemera Thank you for your response! I wonder - why is multi-occur not using completing-read-multiple already internally? Should this potentially be fixed in emacs upstream? As far as I understand completing-read-multiple is part of the Emacs api, which is enhanced by selectrum?

@minad
Copy link
Contributor Author

minad commented Nov 7, 2020

@clemera After looking at multi-occur, I think it should just be replaced by the following:

(defun multi-occur (bufs regexp &optional nlines)
  (interactive (cons
                (mapcar #'get-buffer
                        (completing-read-multiple "Buffer: "
                                                  #'internal-complete-buffer))
                (occur-read-primary-args)))
  (occur-1 regexp nlines bufs))

or alternatively

(defun my-multi-occur (bufs regexp &optional nlines)
  (interactive (cons
                (mapcar #'get-buffer
                        (completing-read-multiple "Buffer: "
                                                  #'internal-complete-buffer))
                (occur-read-primary-args)))
  (multi-occur regexp nlines bufs))

This should probably go to the wiki or we should consider upstreaming the improved multi-occur?

EDIT: For reference the original code of multi-occur in replace.el is the following:

(defun multi-occur (bufs regexp &optional nlines)
  (interactive
   (cons
    (let* ((bufs (list (read-buffer "First buffer to search: "
				    (current-buffer) t)))
	   (buf nil)
	   (ido-ignore-item-temp-list bufs))
      (while (not (string-equal
		   (setq buf (read-buffer
			      (if (eq read-buffer-function #'ido-read-buffer)
				  "Next buffer to search (C-j to end): "
				"Next buffer to search (RET to end): ")
			      nil t))
		   ""))
	(cl-pushnew buf bufs)
	(setq ido-ignore-item-temp-list bufs))
      (nreverse (mapcar #'get-buffer bufs)))
    (occur-read-primary-args)))
  (occur-1 regexp nlines bufs))

@clemera
Copy link
Collaborator

clemera commented Nov 7, 2020

Yes, adding it to the wiki would be great! M-x report-emacs-bug is also a good idea, though looking at the ido variables I guess it needs to be updated in a way that still supports calling it with ido and I think it doesn't support completing-read-multiple but not sure about that.

@minad
Copy link
Contributor Author

minad commented Nov 7, 2020

@clemera Thank you - I will add this to the wiki for now! Regarding upstreaming - I will look into this when I have time, but I cannot make any promises here.

@minad minad closed this as completed Nov 7, 2020
@clemera
Copy link
Collaborator

clemera commented Nov 7, 2020

Great, thanks!

@raxod502
Copy link
Member

Just in case you didn't know, you can also use selectrum-submit-exact-input (bound by default to C-j in the minibuffer) to exit with the empty string, rather than using the up arrow to select the prompt area and pressing RET. This might be more convenient for dealing with situations like these.

@raxod502 raxod502 removed the waiting on response Needs more info or follow-up, will be closed after 90 days if no response label Dec 25, 2020
@hmelman
Copy link

hmelman commented Apr 19, 2021

Any conclusions if this should be reported as a bug in Emacs that multi-occur doesn't use completing-read-multiple?

@minad
Copy link
Contributor Author

minad commented Apr 19, 2021

@hmelman If you want you can create a bug report and attach a patch based on consult-multi-occur (or just use the code from above). I don't want to start too many different discussions on emacs-devel right now ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants