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

Correct highlighting for multiple regexps #1587

Closed

Conversation

raxod502
Copy link
Contributor

@raxod502 raxod502 commented May 23, 2018

Fixes #654.

This was a really simple fix. (Only the let* forms and the dolist at the beginning were touched.) So maybe I am missing something?

It may also be necessary to adapt ivy--subexps to handle a list of integers, so that each regexp in the list may have a different number of subexpressions. That could be doable in a separate commit, depending on how important it is.

@raxod502 raxod502 changed the title swiper.el: highlight multiple regexps correctly Correct highlighting for multiple regexps May 23, 2018
ivy.el Outdated
(cl-remove-if
(lambda (regexp-cons)
(null (cdr regexp-cons)))
ivy--old-re))
Copy link
Collaborator

@basil-conto basil-conto May 27, 2018

Choose a reason for hiding this comment

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

Why not

(cl-mapcan (lambda (cell)
             (and (cdr cell)
                  (list (car cell))))
           ivy--old-re)

or, at the very least,

(mapcar #'car (cl-remove-if-not #'cdr ivy--old-re))

(Guaranteed 100% untested, mind you.)

swiper.el Outdated
(cl-remove-if
(lambda (regexp-cons)
(null (cdr regexp-cons)))
regexp-or-regexps))
Copy link
Collaborator

@basil-conto basil-conto May 27, 2018

Choose a reason for hiding this comment

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

Ditto.

@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented May 27, 2018

Fixed, thanks.

@basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented May 31, 2018

Does this relate to #1550 and #1551 at all?

@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented May 31, 2018

I only looked at them briefly, but I think this pull request solves both of those issues. Namely, all subparts of the search query are highlighted in the minibuffer, without having to update ivy-highlight-functions-alist, and match groups are also highlighted both in the buffer and minibuffer.

I used both functionalities in prescient.el:

image

@basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented May 31, 2018

I only looked at them briefly

As did I, hence the lazy question. :)

but I think this pull request solves both of those issues.

Happy days! Even if this PR doesn't iron out their every kink, you can probably still mention them in the commit message.

@raxod502 raxod502 force-pushed the feat/swiper-multiple-regexps branch from c827477 to b4fac8b Compare May 31, 2018
@raxod502
Copy link
Contributor Author

@raxod502 raxod502 commented May 31, 2018

OK, done :)

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Jun 13, 2018

Thanks.

@raxod502 raxod502 deleted the feat/swiper-multiple-regexps branch Jun 13, 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.

3 participants