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

Add ivy-rotate-preferred-builders to switch re builders #1094

Closed
wants to merge 1 commit into from

Conversation

glucas
Copy link
Contributor

@glucas glucas commented Jul 3, 2017

Fixes #1093.

'ivy--regex-fuzzy)
"fuzzy"
"ivy"))
"Return description of `ivy--regex-function'. "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Trailing space?

"Return description of `ivy--regex-function'. "
(let ((cell (assoc ivy--regex-function ivy--preferred-re-builders)))
(if cell
(cdr cell)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be allowed through as is, or truncated past 5 columns (see hydra hint below)? Is it worth mentioning the limit anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would keep the formatting out of this function, since conceivably users might use this display name in other ways. The hydra-ivy hint is formatted with a min width of 5 padded on the right. A longer string here does mean the last column doesn't column won't line up but it still seems readable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

conceivably users might use this display name in other ways

By display name do you mean that given in ivy--preferred-re-builders, or the value returned by ivy--matcher-desc? In the latter case I would argue it's a non-user-facing function specific to the hydra hint's formatting, and so a reasonable place to apply formatting transformations. Not that I have strong feelings about this - just confirming consensus.

(ivy--regex-ignore-order . "order")
(ivy--regex-fuzzy . "fuzzy"))
"Alist of preferred re-builders with display names.
This list can be rotated with `ivy-rotate-preferred-builders'."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically ivy-rotate-preferred-builders doesn't modify this list - only ivy--regex-function is "slid" across it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, the doc is misleading. I'll fix this.

(setq ivy--old-re nil)
(setq ivy--regex-function
(let ((cell (assoc ivy--regex-function ivy--preferred-re-builders)))
(car (or (cadr (memq cell ivy--preferred-re-builders))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The call to memq should always work because cell is a reference into the same alist that memq is operating on, but I think that this is not very clear and a bit of a hack. I suggest either using member or, better yet, passing (car cell) to memq or member, the latter for symmetry with the use of assoc.

@abo-abo abo-abo closed this in ae438ff Jul 4, 2017
@abo-abo
Copy link
Owner

abo-abo commented Jul 4, 2017

Thanks. It's a bit weird with swiper since it uses swiper--re-builder, but it still works.

@glucas glucas deleted the fix-1093 branch July 4, 2017 14:15
@@ -3536,6 +3536,25 @@ Don't finish completion."
(insert (substring (ivy-state-current ivy-last) 0 -1))
(insert (ivy-state-current ivy-last))))

(defcustom ivy--preferred-re-builders
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find it a bit contradictory for a user option to have a name with double hyphens.

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