git-auto-commit-mode icon indicating copy to clipboard operation
git-auto-commit-mode copied to clipboard

Add automatic pull support

Open kephale opened this issue 3 years ago • 5 comments

I have been happily using git-auto-commit-mode, but when the git repo is being worked on separately from different locations then I get merge conflicts when trying to auto push. This is a bit of a nuisance when doing something like working in a directory of org files and the conflicts are automatically mergable but it is easy to not notice that there was a merge conflict (plus it still requires manual resolution).

This PR introduces automatic pull support which runs in a before-save-hook. It works the same way as auto push, but simply pulls before the buffer is saved. It is also labeled as risky.

kephale avatar Sep 22 '21 09:09 kephale

Hey!

Thanks for this pull request :) I'm glad to hear that you're found git-auto-commit-mode to be a useful project. The code basically looks good, I just have one question which I'll post inline.

ryuslash avatar Oct 05 '21 05:10 ryuslash

Phew, sorry this took so long, but there were some subtle issues that required a continuous block of time to solve them all at the same time. Note that this looks quite a bit different than the previous iteration.

One annoying thing I introduced is that since it uses a make-thread call this blocks for a bit on save, which can be frustrating. I'm not sure what the async version of this is.

[edit] it looks replacing make-thread with async-start from https://melpa.org/#/async solves it, but I'm not sure how you feel about the extra dependency [/edit]

kephale avatar Nov 11 '21 01:11 kephale

Hey :) That's alright, we all have busy lives and nobody is getting paid to work on this, things take time. And this doesn't seem like an easy issue to fix (after making changes, before saving, quickly pull changes which may affect the file you're trying to save).

I don't think the problem is the call to make-thread. I haven't used it in Emacs before, so I had to look up how it works a little. It says it's "mostly cooperative", which took me a moment or two to parse. Basically, because the lambda you created at no point is waiting for input from the keyboard, calling accept-process-output, or thread-join, it's not going to provide any benefit over calling the lambda without a thread. I'm not quite sure how start-process, accept-process-output, and make-thread are supposed to work together, since accept-process-output sounds like it returns either nil or non-nil depending on whether any output was received or not, not the output that may or may not have been received. Is it something like this? (using the previous implementation for push as example)

(let ((process (start-process "git" "*git-auto-push*" "git" "push")))
  (set-process-filter process #'gac-process-filter)
  (set-process-sentinel process #'gac-process-sentinel)
  (while (process-live-p process)
    (accept-process-output process)))

If async solves this in a clean way and works with password input by all means feel free to add it.

ryuslash avatar Nov 22 '21 04:11 ryuslash

I'm not sure how helpful this is, but if you haven't come across it you might want to check out how git-sync handles merge conflicts and so on. It's a bash script and I've been using it across multiple computers for quite a while. It does a good job of automatic pushing and warning on merge conflicts.

chasecaleb avatar Nov 22 '21 16:11 chasecaleb

Hi, I'm also using this minor mode with pleasure. To solve the problem with auto pulling before pushing, I use the following in my Doom Emacs config:

(use-package! git-auto-commit-mode
  :config
  (setq-default gac-automatically-push-p t)
  (setq-default gac-automatically-add-new-files-p t)

  (defun gac-pull-before-push (&rest _args)
    (let ((current-file (buffer-file-name)))
      (shell-command "git pull")
      (when current-file
        (with-current-buffer (find-buffer-visiting current-file)
          (revert-buffer t t t)))))
  (advice-add 'gac-push :before #'gac-pull-before-push))

Should be working regardless of Doom. I only had to initially set the pull strategy.

siatko avatar Oct 20 '23 07:10 siatko