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

counsel-git-grep highlights matches in filenames #483

Closed
hmelman opened this Issue Apr 21, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@hmelman

hmelman commented Apr 21, 2016

counsel-git-grep lists matching lines prefixed with the filename and line number (as it should). It also highlights the matching strings in the lines shown (I believe via ivy--format-minibuffer-line). However it might also highlight part of the filename if it matches the regexp. I don't think it should highlight the filename (or line number) part of the line because that wasn't why the line is considered to match the regexp.

I assume similar functions such as counsel-ag and counsel-pt have the same problem.

This came from my work in #395 but I never came up with a good solution.

@abo-abo abo-abo closed this in ab795d0 Apr 22, 2016

@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Apr 22, 2016

Owner

Thanks.

Owner

abo-abo commented Apr 22, 2016

Thanks.

@hmelman

This comment has been minimized.

Show comment
Hide comment
@hmelman

hmelman Apr 22, 2016

Thanks for speedy fix but it seems rather specific. Does it work for anything else? Rather than checking if the caller was counsel-git-grep could there be an optional argument that could be passed in with a regexp or a function defining which part of the line to highlight?

hmelman commented Apr 22, 2016

Thanks for speedy fix but it seems rather specific. Does it work for anything else? Rather than checking if the caller was counsel-git-grep could there be an optional argument that could be passed in with a regexp or a function defining which part of the line to highlight?

@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Apr 22, 2016

Owner

I don't want to be changing the arglists too much, checking :caller is the way to go for now. Just remains to add counsel-ag to that list.

Owner

abo-abo commented Apr 22, 2016

I don't want to be changing the arglists too much, checking :caller is the way to go for now. Just remains to add counsel-ag to that list.

@hmelman

This comment has been minimized.

Show comment
Hide comment
@hmelman

hmelman Apr 22, 2016

and counsel-pt :) Would be nice for users to be able to add to that list too, maybe a defcustom?

It seems wrong to have a check for a counsel-level command in ivy.el, but I see it's not the only one. ivy-occur-revert-buffer has a check for (memq caller '(counsel-git-grep counsel-grep counsel-ag)). It seems counsel-pt should be added to that list and it's docstring is mistaken. Maybe it's that counsel.el should have a defcustom for counsel-grep-like-commands and ivy could check for that. At least that way, adding a new command could be limited to changes in counsel.el.

hmelman commented Apr 22, 2016

and counsel-pt :) Would be nice for users to be able to add to that list too, maybe a defcustom?

It seems wrong to have a check for a counsel-level command in ivy.el, but I see it's not the only one. ivy-occur-revert-buffer has a check for (memq caller '(counsel-git-grep counsel-grep counsel-ag)). It seems counsel-pt should be added to that list and it's docstring is mistaken. Maybe it's that counsel.el should have a defcustom for counsel-grep-like-commands and ivy could check for that. At least that way, adding a new command could be limited to changes in counsel.el.

abo-abo added a commit that referenced this issue Apr 22, 2016

ivy.el (ivy--format-minibuffer-line): Update
Add "counsel-ag counsel-pt" to the special highlight behavior.

Re #483
@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Apr 22, 2016

Owner

I'll think about adding an API for this at some point. But I'm not sure anyone would want to deal with the logic: you have to pretty much replace ivy--format-minibuffer-line with something just as complex in a user-level ivy-read call.

On the other hand, it's just a str->str mapping, not rocket science. And helm has as similar API for this, if I recall correctly.

Owner

abo-abo commented Apr 22, 2016

I'll think about adding an API for this at some point. But I'm not sure anyone would want to deal with the logic: you have to pretty much replace ivy--format-minibuffer-line with something just as complex in a user-level ivy-read call.

On the other hand, it's just a str->str mapping, not rocket science. And helm has as similar API for this, if I recall correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment