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

Generalize swiper-all function for different buffer lists #2856

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

apella
Copy link

@apella apella commented Apr 28, 2021

Recently a colleague asked me if it was possible to get the same
behaviour as swiper-all with only the buffers of the current
project.

The solution I wrote for him involved a copy of swiper-all-function
as the list of buffers was not variable. I think this is something
that might be useful for others so here's half of that work to allow
others to do the same: search through a custom list of buffers with
all the niceties of swiper-all.

I haven't done the paperwork for the copyright assignment to the FSF
yet so I can't provide swiper-projectile. When I have the paperwork
done I can make a separate PR for that if people are interested.

Recently a colleague asked me if it was possible to get the same
behaviour as `swiper-all` with only the buffers of the current
project.

The solution I wrote for him involved a copy of `swiper-all-function`
as the list of buffers was not variable. I think this is something
that might be useful for others so here's half of that work to allow
others to do the same: search through a custom list of buffers with
all the niceties of `swiper-all`.

I haven't done the paperwork for the copyright assignment to the FSF
yet so I can't provide `swiper-projectile`. When I have the paperwork
done I can make a separate PR for that if people are interested.
Copy link
Collaborator

@basil-conto basil-conto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

search through a custom list of buffers with all the niceties of swiper-all

I've never really used this functionality, so I'm curious: where does swiper-multi fit in here? How much overlap is there between that and swiper-all? Are there any opportunities for sharing/merging functionality? Could/should swiper-multi be extended instead of swiper-all?

I haven't done the paperwork for the copyright assignment to the FSF yet so I can't provide swiper-projectile. When I have the paperwork done I can make a separate PR for that if people are interested.

I'd personally be more interested in an implementation that uses the built-in and extensible project.el, which is also available from GNU ELPA and GNU-devel ELPA, rather than the third-party projectile. TIA.

swiper.el Show resolved Hide resolved
(defun swiper-all-function (str)
"Search in all open buffers for STR."
(defun swiper-all-buffers-function (str buffers)
"Search in all given BUFFERS for STR."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Search for STR in all given BUFFERS.

(or
(ivy-more-chars)
(let* ((buffers (cl-remove-if-not #'swiper-all-buffer-p (buffer-list)))
(let* ((buffers (cl-remove-if-not #'swiper-all-buffer-p buffers))
Copy link
Collaborator

@basil-conto basil-conto May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This let* can be a plain let, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I don't see why not

@apella
Copy link
Author

apella commented May 3, 2021

I've never really used this functionality, so I'm curious: where does swiper-multi fit in here? How much overlap is there between that and swiper-all? Are there any opportunities for sharing/merging functionality? Could/should swiper-multi be extended instead of swiper-all?

I haven't really used it either but this question was a good prompt for me to go and learn more about swiper.
swiper-multi doesn't fit this purpose because it means you have to manually select the buffers first. That takes time if you just want to do a quick search in some of the buffers.

swiper-all has some exclusions: With some exceptions, buffers that aren't backed by a file are not searched.
If the two functions were combined and you selected those buffers in swiper-multi then you wouldn't get any results back.

That said, there is some overlap:
swiper-all-action and swiper-multi-action-2 do the same thing but in a slightly different way.
In this case, I think having the different names might actually be helpful. That way, if people change one, they don't break the other.

@basil-conto
Copy link
Collaborator

Thanks for looking into this.

swiper-multi doesn't fit this purpose because it means you have to manually select the buffers first.

But in theory it could be extended to take a list of buffers, just like this PR extends swiper-all - hence my question about the two.

If the two functions were combined and you selected those buffers in swiper-multi then you wouldn't get any results back.

Are you saying that swiper-multi fails on non-file-visiting buffers? If so, that sounds like a bug to me.

swiper-all-action and swiper-multi-action-2 do the same thing but in a slightly different way. In this case, I think having the different names might actually be helpful. That way, if people change one, they don't break the other.

Why not merge them into a single function that can be parameterised to behave in one or the other way?

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

Successfully merging this pull request may close these issues.

2 participants