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

counsel.el (counsel-ag, counsel-rg): store command persistently #1474

wants to merge 1 commit into from


Copy link

@ericdanan ericdanan commented Feb 23, 2018

This adds a new variable counsel-ag-command which stores counsel-ag-base-command including the optional extra-ag-args and is used by counsel-ag-occur. Also simplifies counsel-rg which now simply let-binds counsel-ag-base-command and calls counsel-ag (similarly to counsel-pt and counsel-ack).

This seem to solve issue #777 : counsel-pt and counsel-ack now work consistently when ivy-resume or ivy-occur is called (counsel-rg was not affected by this issue because it was duplicating the code from counsel-ag rather than calling it inside a let-binding).

It also solves issue #1462 which was closed because submitted about counsel-projectile-rg but actually affected counsel-rg and counsel-ag : extra args are now remembered when calling ivy-occur.

A couple of things to note:

  • counsel-rg was not exactly duplicating the code from counsel-ag, there were small differences in the way extra args were read or initial directory was set. I've resolved them in the way that made more sense to me, but maybe they were intentional. Also in counsel-rg, the caller was set to counsel-rg but now it is counsel-ag, I don't know if that's an issue.
  • In a way these changes make counsel-ag more similar to counsel-grep and it may be possible to make one rely on the other (eg counsel-ag-function / counsel-grep-function), but I didn't touch anything there. I noticed that the command used in counsel-grep-occur is different from counsel-grep-base-command, I don't know if that's on purpose.

Let me know if you would like me to make further changes.

Copy link

@abo-abo abo-abo commented Feb 25, 2018

Thanks, I like the simplification.

Copy link

@liebkne liebkne commented May 2, 2018

Is it intentionally to change the original behavior of doing ag from default-directory to .git based directory when initial-directory is nil?

Copy link

@abo-abo abo-abo commented May 2, 2018

@Isolet Yes, this is intentional: I want to grep the whole project 95% of the time. For the other 5%, default-directory can be chosen by using C-u prefix.

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

Successfully merging this pull request may close these issues.

None yet

3 participants