bbatsov / helm-projectile

Helm UI for Projectile
329 stars 71 forks source link

Support searching without automatically setting input #35

Closed dandavison closed 8 years ago

dandavison commented 8 years ago

Fixes https://github.com/bbatsov/helm-projectile/issues/32

This PR makes a backwards-compatible change that allows the user to call helm-projectile-grep and helm-projectile-ack without having the input set automatically.

I wrote the reasoning for this change in #32. I'm going to paste that here.


helm-projectile-grep automatically uses the current symbol at point (if any) as input. This means that if I think of a thing to search for, and don't pay attention to where my cursor is, I find myself in one of two states:

  1. minibuffer already has some input; I need to delete it before entering my search term
  2. minibuffer is empty

I think that it would be useful to provide a way to be certain that you will be in state 2: i.e. a way for the user to say: "regardless of where my cursor might be, do not seed my search with anything, I just want to type it in".

I'm new to these libraries, but, "I just want to type it in" seems to be the contract that the helm-*-grep-* functions offer. However, helm seems not to offer the thing-at-point or current-region-contents functionality at all, so I suggest that helm-projectile should offer all three versions.


This PR does not change the default behavior of helm-projectile. A separate question which could be discussed in future issues/PRs is what the default behavior of helm-projectile should be: helm doesn't default to setting input automatically, so there is an argument that helm-projectile should follow suit.

With this change a user can define custom functions that change the behavior:

(defun my-helm-projectile-grep (&optional use-input)
  (interactive "P")
  (helm-projectile-grep nil (not use-input)))

(defun my-helm-projectile-ack (&optional use-input)
  (interactive "P")
  (helm-projectile-ack nil (not use-input)))
dandavison commented 8 years ago

@bbatsov do you have time to consider this PR?

bbatsov commented 8 years ago

Probably this should be a defcustom instead of an extra param. Seems pretty doubtful to me that anyone but you would define the custom functions you mentioned (because nobody would figure out this is possible).

bbatsov commented 8 years ago

@dandavison ping :-)

dandavison commented 8 years ago

Thanks for the reminder @bbatsov! I've changed the PR to use a defcustom as you suggested: see helm-projectile-set-input-automatically. Feel free to choose a better name.

I've made this PR 100% behavior-preserving. Note that there was a pre-existing slight inconsistency in the behavior of helm-projectile-ag (no auto-input by default) vs. helm-projectile-ack and helm-projectile-grep (auto-input by default). This is documented in the defcustom text. I'll defer to you on whether we want to change any of the default behavior in subsequent PRs.

dandavison commented 8 years ago

@bbatsov ping, we'll get this done eventually!

bbatsov commented 8 years ago

Indeed. :-)