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

ivy.el: More yank pulsing followups #1859

Closed
wants to merge 1 commit into from

Conversation

@basil-conto
Copy link
Collaborator

commented Dec 14, 2018

(ivy-read): Ensure ivy--pulse-overlay is deleted before exiting completion.
(ivy--pulse-region): Add docstring. Only extend existing overlay's region if it is contiguous with the new region (re: #1856 (comment)).
(ivy--pulse-cleanup): Explicitly set ivy--pulse-timer and ivy--pulse-overlay to nil (re: #1856 (comment)).

Re: #1856
Cc: @mookid

@basil-conto

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2018

@abo-abo Is there a more appropriate place to call ivy--pulse-cleanup in ivy-read?

@basil-conto basil-conto force-pushed the basil-conto:blc/pulse branch from d7500fa to a830dbf Dec 14, 2018

ivy.el: More yank pulsing followups
(ivy-read): Ensure ivy--pulse-overlay is deleted before exiting
completion.
(ivy--pulse-region): Add docstring.  Only extend existing overlay's
region if it is contiguous with the new region.
(ivy--pulse-cleanup): Explicitly set ivy--pulse-timer and
ivy--pulse-overlay to nil.

Re: #1856

@basil-conto basil-conto force-pushed the basil-conto:blc/pulse branch from a830dbf to 64f801a Dec 15, 2018

@abo-abo abo-abo closed this in c507485 Dec 17, 2018

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Dec 17, 2018

Thanks.

Is there a more appropriate place to call ivy--pulse-cleanup in ivy-read?

Not sure, let's keep it as is for now.

I would also probably like to have a defcustom to turn the pulse off, with default value being on.

@basil-conto

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 18, 2018

Thanks.

I would also probably like to have a defcustom to turn the pulse off, with default value being on.

At the moment I'm disabling the pulsing by setting ivy-pulse-delay to zero. Would you like to promote this variable to a user option or would you prefer a separate user option?

@basil-conto basil-conto deleted the basil-conto:blc/pulse branch Dec 18, 2018

basil-conto added a commit to basil-conto/swiper that referenced this pull request Dec 18, 2018
ivy.el: Minor yank pulse simplification
(ivy--yank-by): Reindent.
(ivy--pulse-region): Simplify logic by ordering START and END.

Re: abo-abo#1859
abo-abo added a commit that referenced this pull request Dec 18, 2018
@abo-abo

This comment has been minimized.

Copy link
Owner

commented Dec 18, 2018

Would you like to promote this variable to a user option or would you prefer a separate user option?

I've promoted the variable to a defcustom, please have a look.

@basil-conto

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 18, 2018

I've promoted the variable to a defcustom, please have a look.

Thanks, just a couple of comments (I'm AFK at the moment):

  1. The docstring needs a second sentence such as "Nil means disable such highlighting."
  2. The custom type should be number, not float.
@abo-abo

This comment has been minimized.

Copy link
Owner

commented Dec 18, 2018

@basil-conto Thanks

basil-conto added a commit to basil-conto/swiper that referenced this pull request Dec 19, 2018
ivy.el: Minor yank pulse simplification
(ivy--yank-by): Reindent.
(ivy--pulse-region): Simplify logic by ordering START and END.

Re: abo-abo#1859
abo-abo added a commit that referenced this pull request Dec 19, 2018
ivy.el: Minor yank pulse simplification
(ivy--yank-by): Reindent.
(ivy--pulse-region): Simplify logic by ordering START and END.

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