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

swiper.el (swiper--ivy) Add integration with evil s expressions #1406

Closed
wants to merge 1 commit into from

Conversation

@YourFin
Copy link
Contributor

commented Jan 11, 2018

I.e. searching for apple with swiper and then running
:s//orange/g will replace all apples on the line with oranges as
expected. Evil uses the value of `isearch-string' for this
completion, hence it's inclusion.

swiper.el (swiper--ivy) Add integration with evil s expressions
I.e. searching for apple with swiper and then running
`:s//orange/g` will replace all apples on the line with oranges as
expected. Evil uses the last value of `isearch-string' for this
completion.
@YourFin YourFin referenced this pull request Jan 11, 2018
@manuel-uberti
Copy link
Contributor

left a comment

Minor nit-picking: is it necessary to have the comment on three different lines?

@abo-abo abo-abo closed this in 6da6d70 Jan 12, 2018

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jan 12, 2018

Thanks. I adjusted your commit a bit. I'm pretty sure ivy-text never has any string properties.

@YourFin

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2018

Looking back at why I had that in, I originally had this on some advice to swiper, and used swiper-history as the source, which does have text properties that it manages to acquire somewhere; I thought that was part of ivy, but apparently not.

Thanks again for your wonderful work!

@edkolev

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

@YourFin the code which does this for evil-mode's evil-search module is here, in swiper--action: https://github.com/YourFin/swiper/blob/fbc72a9a6dd1cc35ae855a2cac65eecacbcb5d01/swiper.el#L746

I would suggest you move the newly added code in swiper--action and maybe tweak it a bit:

(when (and (bound-and-true-p evil-mode) (eq evil-search-module 'isearch))
        (setq isearch-string (substring-no-properties ivy-text)))

As a result, all the code dealing with evil's search integration (be it evil-search or isearch) will be in the same function: swiper--action.

Cheers!

abo-abo added a commit that referenced this pull request Feb 11, 2018
@abo-abo

This comment has been minimized.

Copy link
Owner

commented Feb 11, 2018

@edkolev Thanks, please check if this change is what you had in mind.

@dieggsy

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2018

@abo-abo at least for evil, isearch-string is only used if evil-search-module is 'isearch, so I think it would require a separate conditional (the current one is for when evil-search-module is evil-search

Although, I'm not super sure how isearch works and what isearch-string is generally for, but especially since it's not an evil-specific thing, would it not make sense to set it regardless of evil?

@edkolev

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2018

@edkolev Thanks, please check if this change is what you had in mind.

@abo-abo I just created a PR which but wraps the new code in a (when (eq evil-search-module 'isearch) ...)

The PR also removes the comment about s-expressions since that means a totally different thing. In this scenario, it's referred to vim's :substitute ex command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.