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

Search History Mess and Proposal #1914

Closed
Alexander-Shukaev opened this issue Jan 31, 2019 · 4 comments
Closed

Search History Mess and Proposal #1914

Alexander-Shukaev opened this issue Jan 31, 2019 · 4 comments

Comments

@Alexander-Shukaev
Copy link

@Alexander-Shukaev Alexander-Shukaev commented Jan 31, 2019

Apart from counsel, I frequently use isearch and I like those sharing history. For example, I recently did

(defvaralias 'swiper-history 'regexp-search-ring)

and it proved to be very useful. However, since I use counsel-grep-or-swiper, it turned out that counsel-grep is writing history to counsel-git-grep-history. First question then, why is it counsel-git-grep-history and not something like counsel-grep-history? Shouldn't those be separate? Also counsel-grep-history feels like a less misleading and a more general name.

Secondly, why also not write to the already existing grep-regexp-history for the same sharing purposes? I can't imagine my life without rgrep at times.

This brings me to the following proposal to tidy things up a bit in this space.

  1. For consistency and flexibility reasons, every search command should definitely write its own history. That is the case with counsel-grep and counsel-git-grep-history definitely needs to be sorted out by adding counsel-grep-history.

  2. All the ivy search-related commands should in addition write a common history, e.g. ivy-search-history. It would make it easier to redirect/merge the whole search history from ivy-derived packages with isearch, for example:

    (defvaralias 'ivy-isearch-history 'regexp-search-ring)
    
  3. Let's teach the :history keyword-related functionality to be customizable and apart from each command writing its own private history, to also consider some defcustom ivy-search-history-variables-alist, where each cdr would be a list of additional custom variables, where to write history to for a particular command. That is one could then selectively choose particular set of commands to write their history to regexp-search-ring and/or grep-regexp-history and/or whatever other history variables.

@abo-abo abo-abo closed this in 050b4a4 Feb 6, 2019
@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Feb 6, 2019

Thanks. I've added counsel-grep-history since it solves the immediate problem.

I'm not sure about messing with grep-regexp-history. Also not sure about the grand unified customizable history design. Of course we could make it, but to what purpose? I prefer that the user just says (require 'counsel) rather than having to define all kinds of aliases for history variables.

We can still pick this up as part of a feature that lets the user customize any arguments to ivy-read per-caller.

@Alexander-Shukaev
Copy link
Author

@Alexander-Shukaev Alexander-Shukaev commented Feb 6, 2019

But how would (require 'counsel) solve my initial requirement that I want ivy search commands to share history with grep and isearch and potentially others in future? At the moment, defvaralias hack is the only solution.

@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Feb 6, 2019

What I meant here is that the user that doesn't want to do extra config should just (require 'counsel) and have all grep-like history shared automatically, like counsel-rg and counsel-git-grep already do.
If we have individual history for every single command, than it's up to either us or the user to group them together via defvaralias (which is not a hack in any bad meaning of the word, I think).

@Alexander-Shukaev
Copy link
Author

@Alexander-Shukaev Alexander-Shukaev commented Mar 12, 2019

counsel-*g and counsel-recoll still use counsel-git-grep-history. They should either have separate histories or at least counsel-grep-history.

astoff added a commit to astoff/swiper that referenced this issue Jan 1, 2021
* counsel.el (counsel-grep): Now has its own history.

Fixes abo-abo#1914
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants