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

Swiper doesn't handle ! properly #1418

Closed
bbatsov opened this issue Jan 24, 2018 · 6 comments

Comments

@bbatsov
Copy link

commented Jan 24, 2018

Just see the screenshot:

image

Seems to me ! should have been considered part of the search string.

@manuel-uberti

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2018

It is working for me. Is it happening with emacs -Q as well?

@mookid

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2018

I can reproduce it with make plain.

@basil-conto

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

This is because the default ivy--regex-function, ivy--regex-plus, considers everything after ! to be a negated pattern. See (ivy) ivy--regex-plus.

ivy--regex-plus does not, however, understand escaped exclamation marks, such as "add-class-path\\!, in the same way that ivy--regex-ignore-order does. I don't know whether this is deliberate (@abo-abo ?), but IMO it's a deficiency.

@mookid

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2018

One easy fix is just to change (split-string str "!" t) to (split-string str " !" t), no?

@basil-conto

This comment has been minimized.

Copy link
Collaborator

commented Jan 24, 2018

@mookid No, that would break established behaviour. I think the following would be better:

diff --git a/ivy.el b/ivy.el
index 7c6cbb8..2ff7c35 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2358,7 +2358,7 @@ ivy--regex-plus
   "Build a regex sequence from STR.
 Spaces are wild card characters, everything before \"!\" should
 match.  Everything after \"!\" should not match."
-  (let ((parts (split-string str "!" t)))
+  (let ((parts (ivy--split-negation str)))
     (cl-case (length parts)
       (0
        "")

but this results in the error "Unexpected: use only one !" no longer being shown when the user enters more than one unescaped !.

@abo-abo abo-abo closed this in 7c9d664 Jan 24, 2018

@abo-abo

This comment has been minimized.

Copy link
Owner

commented Jan 24, 2018

Thanks all.

The expected behavior works like this: when input is add path\!, the matched regex is "\\(add\\).*?\\(path!\\)". I've added a unit test for this as well.

abo-abo added a commit that referenced this issue Jan 24, 2018
ivy.el (ivy--highlight-default): Works on negation
For example, `counsel-M-x' with input "^vi!ew" will correctly
highlight all commands that match "^vi" but don't match "ew".

Re #1418
abo-abo added a commit that referenced this issue Jan 24, 2018
swiper.el (swiper--add-overlays): Improve highlights with negation
Example where the change is relevant: in ivy.el, call `swiper' with
input "defun!ivy". There is only one match. Unlike before this change,
"defun" is highlighted only for the match, not anywhere else in the
window.

Another example of useful nagation in ivy.el: "ivy len!--".

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