dandavison / magit-delta

Use delta (https://github.com/dandavison/delta) when viewing diffs in Magit
MIT License
296 stars 10 forks source link

Seems to break staging by hunks #22

Open vermiculus opened 2 years ago

vermiculus commented 2 years ago

When I have magit-delta-mode enabled, I can't seem to stage individual hunks:

image

  1 git … apply --reverse -p0 --ignore-space-change -
error: patch failed: init-custom.el:30
error: init-custom.el: patch does not apply

This seem related to #13 and #18, but I don't have line numbers enabled.

dandavison commented 2 years ago

Hi @vermiculus, let's get to the bottom of this. Staging by hunks does work in general (is working for me; wasn't aware of others having problems before this). Can you post your magit-delta config, and the [delta] section from your ~/.gitconfig?

vermiculus commented 2 years ago

I will check my personal computer when I get home, but I had just installed delta to try this out, so I don't think I have any config unless installation itself created it.

My magit-delta configuration is only what you see in the the use-package form in the screenshot.

vermiculus commented 2 years ago

git config --list --show-origin | grep delta yields nothing.

dandavison commented 2 years ago

@vermiculus would you be able to do a little experiment? I'm thinking, let's capture the args and input that git is receiving so we can see what is wrong with the patch. So instructions would be

  1. Create a file at /tmp/fake-git containing this:

    #!/bin/bash
    OUTFILE=/tmp/git-input
    echo "$@" >>$OUTFILE
    tee -a $OUTFILE | git $@
    echo -------------------------------------------------------------------------- >>$OUTFILE
  2. chmod +x /tmp/fake-git

  3. (setq magit-git-executable "/tmp/fake-git")

Now stage a hunk in magit and look in /tmp/git-input. We should be able to see the arguments, and the patch, that git received (along with various other git arguments). In my case while testing I can see this:

--no-pager --literal-pathspecs -c core.preloadindex=true -c log.showSignature=false -c color.ui=false -c color.diff=false apply --cached -p0 --ignore-space-change -
diff --git src/delta.rs src/delta.rs
index 7fbcdf4..0540103 100644
--- src/delta.rs
+++ src/delta.rs
@@ -268,3 +268,4 @@ fn detect_source(line: &str) -> Source {
         Source::Unknown
     }
 }
+hello

Could you post the args and patch that are written to that file when magit reports that the patch failed?

dandavison commented 2 years ago

Also, just to check -- is delta working correctly outside magit?

Is it just staging individual hunks that is problematic, or are there any other problems when using magit-delta?

vermiculus commented 2 years ago

To be fair, I haven't used delta outside of magit-delta :-) but the diff-washing was visually working fine (at least, as I'd expect without prior experience with delta) from within magit.

Staging individual hunks was the only problem I noticed.

ErnestDong commented 2 years ago

I encountered this when I set line-numbers = true in delta

dandavison commented 2 years ago

@vermiculus this problem seems to be specific to your environment. What would be really helpful is if you could capture the patch that is being sent to git:

error: init-custom.el: patch does not apply

I think if we could see that, we'd see what is going wrong. I've put instructions for capturing the patch above: https://github.com/dandavison/magit-delta/issues/22#issuecomment-1002379108

g5pw commented 2 years ago

I have the same situation, and have line-numbers = true in my config. If I set it to false, everything works.

dandavison commented 2 years ago

Hi @g5pw, that's right: line-numbers cannot be used with magit-delta since it creates invalid patches. The simplest approach is to make magit-delta independent of your command-line delta settings using:

(add-to-list 'magit-delta-delta-args "--no-gitconfig")

And then add the settings you do want in magit-delta using elisp (e.g. more add-to-list calls).

But there are several other possibilities, if you don't like the potential duplication of settings in that solution. E.g. you could use features to define magit-delta and non-magit-delta features and ensure that they are activated accordingly in magit-delta versus your usual command line delta.

g5pw commented 2 years ago

Thanks @dandavison, your profile suggestion resolved the issue! Still, it feels a bit like a workaround, maybe there could be a way to automatically disable line numbers when called from magit?

choma commented 1 year ago

I did what @dandavison suggested:

But there are several other possibilities, if you don't like the potential duplication of settings in that solution. E.g. you could use features to define magit-delta and non-magit-delta features and ensure that they are activated accordingly in magit-delta versus your usual command line delta.

So, in case it is useful for someone, here it is:

;; ~/.emacs.d/init.el
(use-package magit-delta
  :hook (magit-mode . magit-delta-mode)
  :config
  (setq magit-delta-delta-args (append magit-delta-delta-args '("--features" "magit-delta"))))
# ~/.config/git/config
[delta]
    navigate = true  # use n and N to move between diff sections
    side-by-side = true
    # 
    features = default magit-delta

[delta "default"]
    line-numbers = true

[delta "magit-delta"]
    line-numbers = false
# ~/.bashrc
export DELTA_FEATURES=+default