Improve Cross-Platform: Using absolute path to describe `patchfile`.

[langshangyuan]

For the function diff-hl-stage-current-hunk, it invokes the git shell command to use the feature of git apply --cached <path/to/patch/file>. However, I found that it uses the relative path to the current directory, which causes problems to use the function in Microsoft Windows.

I have been tested the same configuration on Linux, the diff-hl-stage-current-hunk function works well. While the function broke on Windows with the error message in minibuffer: vc-do-command: Failed (status 1): git --no-pager apply --cached ../../../AppData/Local/Temp/diff-hl-stage-patchgan72XInvalid. I think change the patchfile path to absolute path (and quote it) will solve the problem.

Besides, the function in Doom Emacs works fine. I think it does many jobs in the tweak of Windows path resolution, environment variable settings.

[dgutov]

Hi!

I think change the patchfile path to absolute path (and quote it) will solve the problem.

Did you have a chance to test that change?

Besides, the function in Doom Emacs works fine.

Which function?

[langshangyuan]

The interactive function diff-hl-stage-current-hunk. It needs to create a temp-file to store the patch information that will be fed to git apply command.

[dgutov]

So Doom has an override for diff-hl-stage-current-hunk? Could you link to it?

[langshangyuan]

I search for it, but I only found a simple wrapper function to change the naming style of the original function. It does not any functionality change.

[dgutov]

Does this help?

diff --git a/diff-hl.el b/diff-hl.el
index ebb19e7..b2fa5d7 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -740,7 +740,7 @@ Only supported with Git."
         (insert (format "diff a/%s b/%s\n" file-base file-base))
         (insert (format "--- a/%s\n" file-base))
         (insert (format "+++ b/%s\n" file-base)))
-      (let ((patchfile (make-temp-file "diff-hl-stage-patch")))
+      (let ((patchfile (expand-file-name (make-temp-file "diff-hl-stage-patch"))))
         (write-region (point-min) (point-max) patchfile
                       nil 'silent)
         (unwind-protect

[langshangyuan]

There is no difference with directly running the command 😞 .

[dgutov]

Perhaps the problem is somewhere else? Does this happen when you try to stage just any hunk?

[langshangyuan]

I don't quite clear whether this package need extra settings. I create a new configuration folder with only the straight.el and diff-hl packages and I don't change any default settings of Emacs, the stage function still returns the same failure message as before.

[dgutov]

It shouldn't need extra settings.

Could you try visiting the generated patch file from the message and see whether its contents are reasonable? Or perhaps you don't have diff installed, something like that.

[langshangyuan]

I search the temp directory in my computer. There is no patchfile in that directory. I think it should be deleted after the execution. But it's quite wired. Maybe the file can't be created successfully. I think this is a quite strange problem. I decide to give up this functionality 😫. Anyway, thank you for your attention and helpful reply!

[dgutov]

If you have the time, try applying this change:

diff --git a/diff-hl.el b/diff-hl.el
index ebb19e7..d890ad5 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -750,7 +750,9 @@ Only supported with Git."
                                 patchfile
                                 "apply" "--cached" ))
               (setq success t))
-          (delete-file patchfile))))
+          (message "Patch file at: %S" patchfile)
+          ;(delete-file patchfile)
+          )))
     (when success
       (if diff-hl-show-staged-changes
           (message (concat "Hunk staged; customize `diff-hl-show-staged-changes'"

and re-evaluate the function. This will make it not delete the patch anymore. Then you could visit it and examine the contents.

[langshangyuan]

Yes! I can find the patchfile now. But I guess the content of it. I guess the content is appropriate.

diff a/"Home.org" b/"Home.org"
--- a/"Home.org"
+++ b/"Home.org"
@@ -63,7 +65,7 @@
 
 ** Life
 
-*** English
+*** [[file:Life/University.org][University]]
 
 [[file:Life/sustainable_future.org][Sustainable Future]]
 [[file:Life/Optimization.org][Optimization]]

I manually execute the git apply command in the evil Ex command mode. And, I get the error message "Home.org": does not exist in the index. Sorry that I am not quite familiar with the git apply command.

[dgutov]

Huh. The quotes around Home.org look odd.

Try this patch, it could fix the problem:

diff --git a/diff-hl.el b/diff-hl.el
index ebb19e7..b3918ab 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -719,7 +719,7 @@ Only supported with Git."
          (file buffer-file-name)
          (dest-buffer (get-buffer-create " *diff-hl-stage*"))
          (orig-buffer (current-buffer))
-         (file-base (shell-quote-argument (file-name-nondirectory file)))
+         (file-base (shell-quote-argument (file-name-nondirectory file) t))
          success)
     (with-current-buffer dest-buffer
       (let ((inhibit-read-only t))

[langshangyuan]

I try this solution, Emacs reports that: let*: Wrong number of arguments: shell-quote-argument, 2. After I directly remove the shell-quote-argument, the stage-hunk function works finally. And if I encounter filename containing special characters like spaces, underscores and so on, I do a small test, the stage function still works. 👍 Are there better solutions compared to directly removing the shell-quote-argument.

[dgutov]

I try this solution, Emacs reports that: let*: Wrong number of arguments: shell-quote-argument, 2

Ah right, it only has the new argument in Emacs 29+.

After I directly remove the shell-quote-argument, the stage-hunk function works finally.

Very good. Yeah, it actually needs a different kind of quoting, and only in specific circumstances. I'll drop the call for now, this should work for most people. Thanks for testing.