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

Pulse when yanking #1581

Closed
wants to merge 1 commit into from

Conversation

@hotpxl
Copy link
Contributor

commented May 19, 2018

ivy-yank-word is a great feature. I often use it with counsel-projectile-ag to search for a symbol in a project. But when the focus changes to Ivy, it's hard to follow the cursor in the original window (especially for people with cursor-in-non-selected-windows set to nil).

I don't know if Helm did this, but I added the pulse of the region that was yanked. It helps me see immediately what I've just yanked into Ivy, without having to switch eye focus back and forth between original document and Ivy.

@basil-conto

This comment has been minimized.

Copy link
Collaborator

commented May 20, 2018

FWIW, I'm a bit squeamish about using CEDET features, even if they are built-in; I would have expected/hoped its general purpose features be extracted as separate libraries.

More importantly, however, this pulsating behaviour breaks convention with and isn't that compatible with swiper highlighting. When I say "breaks convention", I mean that it doesn't apply the same style of in-buffer highlighting as Swiper. When I say "incompatible", I mean that pulse.el overlays are hidden by swiper overlays, which makes the feature less clear/effective when used with swiper.

As such, I think pulse.el should be loaded as needed, ideally based on a user option, instead of unconditionally being made a top-level dependency of ivy.el. I, for one, would prefer Swiper-like highlighting for counsel-projectile-ag, rather than pulsating overlays.

@hotpxl

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2018

OK. I'm not particular about whether we use overlay or not. I'm just wondering if I can get the same "flickering" behavior using a compatible highlighting method. I don't really want a persistent highlight like that in a search.

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2018

@hotpxl Sorry for the late reply, the pull request seems to be out of date. Could you update it?

Like, @basil-conto, I don't like using CEDET features here. Maybe we can think of something equivalent as necessary.

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Dec 12, 2018

@hotpxl Thanks for the idea.

basil-conto added a commit to basil-conto/swiper that referenced this pull request Dec 13, 2018
ivy.el: Touch up yank pulsing
(ivy-pulse-delay): New variable.
(ivy--pulse-timer): Add markup to docstring.
(ivy--pulse-region): If ivy--pulse-overlay still exists from the
previous invocation, expand its region, rather than moving it.
Remove redundant call to move-overlay.  Use ivy-pulse-delay.
(ivy--pulse-cleanup): New function for ensuring ivy--pulse-timer and
ivy--pulse-overlay are disposed of.

Re: abo-abo#1581, abo-abo#1850
abo-abo added a commit that referenced this pull request Dec 13, 2018
ivy.el: Touch up yank pulsing
(ivy-pulse-delay): New variable.
(ivy--pulse-timer): Add markup to docstring.
(ivy--pulse-region): If ivy--pulse-overlay still exists from the
previous invocation, expand its region, rather than moving it.
Remove redundant call to move-overlay.  Use ivy-pulse-delay.
(ivy--pulse-cleanup): New function for ensuring ivy--pulse-timer and
ivy--pulse-overlay are disposed of.

Re: #1581, #1850
Fixes #1856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.