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: Drop preselected candidate after input #1573

Closed

Conversation

raxod502
Copy link
Contributor

@raxod502 raxod502 commented May 15, 2018

Suggested fix for #1563.

ivy.el Outdated
;; If there was a preselected candidate, don't try to
;; keep it selected even if the regexp still matches
;; it. See issue #1563. See also `ivy--preselect-index',
;; which this logic roughly mirrors.
Copy link
Collaborator

@basil-conto basil-conto May 15, 2018

Choose a reason for hiding this comment

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

Nitpick: please honour the sentence-end-double-space setting in dir-locals-file.

ivy.el Outdated
(and (stringp (ivy-state-preselect ivy-last))
(string-match-p
(ivy-state-preselect ivy-last)
(ivy-state-current ivy-last)))))
Copy link
Collaborator

@basil-conto basil-conto May 15, 2018

Choose a reason for hiding this comment

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

Nitpick: might be nice to bind ivy-state-preselect and ivy-state-current to reduce visual clutter (and avoid duplicate calls).

Copy link
Collaborator

@basil-conto basil-conto May 15, 2018

Choose a reason for hiding this comment

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

(Seeing as there are other calls to these accessors in the function as well.)

@raxod502 raxod502 force-pushed the feat/override-preselected-on-input branch from eb4bf6a to 1a44976 Compare May 15, 2018
@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented May 15, 2018

@basil-conto Good ideas! I made the relevant changes.

ivy.el Outdated
;; See issue #1563. See also `ivy--preselect-index',
;; which this logic roughly mirrors.
(not (let ((preselect (ivy-state-preselect ivy-last))
(current (ivy-state-current ivy-last)))
Copy link
Collaborator

@basil-conto basil-conto May 15, 2018

Choose a reason for hiding this comment

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

Nitpick: Again, there are other parts of ivy--recompute-index which could make use of these bindings.

@raxod502 raxod502 force-pushed the feat/override-preselected-on-input branch from 1a44976 to 01c2dda Compare May 21, 2018
@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented May 21, 2018

@basil-conto Fixed, thanks.

@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented Jul 2, 2018

@abo-abo Any comments?

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Jul 3, 2018

Merged, thanks.

raxod502 added a commit to radian-software/prescient.el that referenced this issue Jul 3, 2018
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.

None yet

3 participants