add after commit hook to magit #603

Closed
steckerhalter opened this Issue Mar 23, 2013 · 11 comments

Projects

None yet

4 participants

@steckerhalter

I'm using diff-hl https://github.com/dgutov/diff-hl and was looking for a way to update all the buffers when I had commited something, I wrote some function and an advice:

(defun diff-hl-update-each-buffer ()
  (interactive)
  (mapc (lambda (buffer)
          (condition-case nil
              (with-current-buffer buffer
                (diff-hl-update))
            (buffer-read-only nil)))
        (buffer-list)))
(defadvice magit-update-vc-modeline (after my-magit-update-vc-modeline activate)
  (progn (diff-hl-update-each-buffer)))

to avoid that it would be nice to have hook, maybe even some way to run a command on every magit buffer after a commit has been made? (comparable to magit-vc-update-vc-modeline)

@tarsius tarsius was assigned May 29, 2013
@tarsius
Member
tarsius commented May 29, 2013

(Note to myself: the hook should be in `magit-refresh-wrapper'.)

@tarsius
Member
tarsius commented Aug 13, 2013

We will start using git-commit-mode instead of magit-log-edit shortly, so I am closing this as wontadd.

Adding a hook somewhere in the refresh machinery is planned. This will however be done in a general way, not specialized as some sort of after-commit-hook.

@tarsius tarsius closed this Aug 13, 2013
@steckerhalter

Hi... is there such a hook now? I can't find it...

@tarsius tarsius reopened this Aug 31, 2013
@tarsius
Member
tarsius commented Sep 2, 2013

Yes, try magit-refresh-file-buffer-hook.

@tarsius tarsius closed this Sep 2, 2013
@steckerhalter

I noticed that this didn't work anymore and digging further I found that magit-refresh-file-buffer-hook has been removed.

I wrote some advice to tackle it:

(defadvice git-commit-commit (after git-commit-commit-after activate)
  (dolist (buffer (buffer-list))
    (with-current-buffer buffer
      (when diff-hl-mode
        (diff-hl-update)))))

not very nice but it works. do you have any other suggestion? or could you add some kind of hook again for this?

@tarsius tarsius added the . label Feb 8, 2014
@bbatsov bbatsov referenced this issue in dgutov/diff-hl Apr 14, 2014
Closed

Magit integration #19

@bbatsov
bbatsov commented Apr 14, 2014

@tarsius I'm having the same problem - I'm using diff-hl, but I need some after commit hook to use it properly with magit. Obviously I can live with an advice, but I'm wondering if there's a better/cleaner way to achieve this.

@tarsius
Member
tarsius commented Apr 14, 2014

I'll look into creating an "after commit" hook.

@tarsius tarsius reopened this Apr 14, 2014
@tarsius tarsius removed 21 wontadd . labels Apr 14, 2014
@tarsius
Member
tarsius commented Apr 15, 2014

A problem git-commit-post-commit-hook would have is that, without further precautions (which likely would be complicated), it is not guaranteed that it would run before magit-refresh (which among other things triggers reverting file-visiting buffers). Also such a hook would be run just once, instead of once per file. I will probably add such a hook eventually but will do it on git-modes's next branch, not directly on master.

Alternative I could run a hook magit-revert-hook once per file (inside the repository) in magit-revert-buffers. However that already runs auto-revert-handler which in turns runs the hook vc-find-file-hook, regardless of whether the file was reverted and provided auto-revert-check-vc-info is non-nil. The default value of that variable is nil but I am considering overriding that using a let-binding around the call to auto-revert-handler.

Could you please try that hook for now. Also please post your magit related hook functions and advices here, so that I can better determine whether I should add new hooks or whether better documenting existing hooks is enough. Thanks.

@dgutov
dgutov commented Apr 15, 2014

vc-find-file-hook is not a hook, it's just a function, which unfortunately doesn't run any hooks in turn, which I might've been able to hook into, AFAICS.

Ideally, any new hook you would add will run after the call to auto-revert-handler, then we'll be able to take advantage of the newly cached vc state information (when it's updated).

@tarsius
Member
tarsius commented Apr 16, 2014

magit-revert-buffers now runs magit-revert-buffer-hook for each of the appropriate file-visiting buffers. To be on the safe side wrap your hook functions body with (when (buffer-file-name) ...). We might, or might not, use that for other buffers too in the future. When that is decided I will also explicitly define the hook variable and add documentation. This hook does more or less what magit-refresh-file-buffer-hook used to do, which probably got removed when we started to rely on library autorevert (which came with a messy transition).

I haven't added an after-commit-hook yet, and thinking about it again I tend to come to the same conclusion as almost a year ago: a after-refresh-hook would be more useful, and also easier to do.

@tarsius tarsius closed this Apr 16, 2014
@dgutov dgutov added a commit to dgutov/diff-hl that referenced this issue Apr 16, 2014
@dgutov dgutov Use magit-revert-buffer-hook
Closes #19
References magit/magit#603
a311beb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment