counsel.el (counsel-git-grep-switch-cmd): allow recursive minibuffer

[brownts]

Fixes #2720.

Locally sets enable-recursive-minibuffers so that if this is not set in the user's environment, it won't cause an error when attempting to configure the git-grep command.

[basil-conto]

Why here and not in counsel-git-grep?

[brownts]

@basil-conto, I was attempting to follow precedent of how this was applied in the past. For example, enable-recursive-minibuffers is also set in counsel-cd, counsel-git-grep-query-replace, etc. To your point, those technically didn't have to be set there and instead could have been set higher at the counsel-ag and counsel-git-grep level. I'm guessing the reason it was done this way in the past is that the setting was performed local to where the need was. In other words, enable-recursive-minibuffers is not required to be set at the counsel-ag or counsel-git-grep level for basic functionality, as it typically works fine there. It's only when something is invoked from their key maps which might need to use the minibuffer again (such as counsel-cd or in this case counsel-git-grep-switch-cmd).

[basil-conto]

I'm not sure how you arrived at this precedent, since there are several top-level Counsel commands that bind the user option. My impression is that the addition of all of these bindings has been ad-hoc and not systematic until now. Ideally Ivy would solve this problem once and for all, rather than requiring us to exhaustively list all places that might need recursive completion.

[brownts]

Maybe the precedent was just ad-hoc then, as you say...it was more of a guess/observation. I don't have a problem moving this up to the counsel-git-grep level. Although, that brings up the bigger question of whether this should be applied at counsel-ag as well and the settings removed from counsel-cd and counsel-git-grep-query-replace. Looking in counsel.el, there are 7 other usages of this binding in the following locations:

• counsel-describe-variable
• counsel-describe-function
• counsel-describe-symbol
• counsel-info-lookup-symbol
• counsel-git-grep-query-replace
• counsel-cd
• counsel-minibuffer-history

In your opinion, of the above list, is it just counsel-cd and counsel-git-grep-query-replace that are incorrectly placed? I can remove them from these locations and instead add the setting to both counsel-ag and counsel-git-grep.

[basil-conto]

Until a system is put in place, I'd say you can leave existing bindings where they are, especially since they're relied on by different callers.

counsel-git-grep-switch-cmd, on the other hand, is more closely tied to counsel-git-grep, so the choice of what to do with the binding is more straightforward. My vote is for putting it in counsel-git-grep, for the benefit of all functions it ends up invoking.

[brownts]

Understood, thanks for the feedback @basil-conto. I'll move the change to counsel-git-grep until a better system is put in place.

[basil-conto]

I think counsel--git-grep-cmd-and-proj can use the minibuffer, so maybe enable-recursive-minibuffers should come first?

[brownts]

Good point, I'll move it up. Thanks.

[basil-conto]

Thanks!