Allow projectile-find-file-hook to be executed on a buffer which is not yet in projectile-mode

[bcoconni]

The current code prevent the hooks to be called if the current buffer minor mode is not already projectile-mode. This is a problem since I meant to implement a hook to switch the new opened buffer to projectile mode if it was opened by [helm-]projectile-find-file in the first place.

In addition there is currently an advice for helm-projectile-find-file to call its hooks but it is modifying an external package (helm-find-file-or-marked) which means that it can potentially have side effects outside helm-projectile.

So this pull request propose another means to achieve the same result: the advice is added to helm-projectile-find-file itself. As result the condition on the current buffer being already in projectile-mode is no longer needed and it keeps the call to the hooks internal to the helm-projectile package.

The pull request also removes the advice added to the global function helm-find-file-or-marked as it would make the hooks to be called twice.

Finally the pull request applies the same modification to helm-projectile-find-dir so that it calls its hooks.

[xiongtx]

I don't think this is correct.

helm-projectile-find-{file, dir} are Helm commands, which open a buffer of candidates and wait for the user to select one (or more) to which to apply some action. We don't want to run these hooks after opening the Helm buffer; we want to run them after the action of opening files/dirs.

This is the purpose of advising helm-find-files-or-marked, which is the action in helm-projectile-file-actions (via helm-find-files-actions) that actually opens the file(s).

[bcoconni]

@xiongtx it seems you missed the point of the code at stake. Indeed it was introduced by the pull request #25 because projectile-find-file-hook was not called when opening a file with helm-projectile-find-file. @Kungsgeten initial proposal was to duplicate the helm code in helm-projectile but @bbatsov suggested to advice helm-find-files-or-marked instead. AFAICT the motivation was really to have simpler code and was not as involved as you are suggesting.

My proposal is to no longer advice an helm function but rather to advice internal helm-projectile functions instead. The result would be exactly the same as with the current code but we would no longer need to check that projectile-mode is enabled before calling the hooks. And that is the motivation of my pull request.

In addition, helm-projectile current code is not calling projectile-find-dir-hooks at all so the topic remains open if you discard my pull request.

Could you please reconsider your position ?

Thanks.

[xiongtx]

Try this:

(defun helm-test ()
  (interactive)
  (helm :sources (helm-build-sync-source "foo"
                   :candidates '(a b c))
        :buffer "*helm test*"))

(advice-add 'helm-test :after
            (lambda (&optional arg)
              ;; No `arg' is passed in
              (message arg)
              ;; This is done even if we `C-g' from `helm-test'
              (message "This is printed even if no candidate is selected.")))

Do you see the problem now?

[xiongtx]

Ping @bcoconni.

[xiongtx]

Closing for now, as I don't think this is the right approach.