wfxr / forgit

:zzz: A utility tool powered by fzf for using git interactively.
MIT License
4.32k stars 136 forks source link

Git diff not displaying in the preview #245

Closed keatsfonam closed 1 year ago

keatsfonam commented 1 year ago

Check list

Environment info

Problem / Steps to reproduce

gd is not showing anything in the preview pane. The file is detected. All other commands are correctly displaying the preview pane

Seems that it is similar to https://github.com/wfxr/forgit/issues/203 which was closed. There are no spaces in my filename.

To reproduce:

Here is gd Screen Shot 2022-11-03 at 12 06 06 PM

Here is git diff Screen Shot 2022-11-03 at 12 06 27 PM

Here is glo properly displaying Screen Shot 2022-11-03 at 12 06 57 PM

carlfriedrich commented 1 year ago

@keatsfonam Thanks for your bug report. I actually cannot reproduce this with zsh on Linux, gd works fine here with your proposed steps:

grafik

Did this use to work in former versions of forgit? If so, can you maybe bisect this to a specific forgit version?

carlfriedrich commented 1 year ago

@keatsfonam Ah, from your screenshot I see that there's obviously a bug in the file list itself, which explains why the preview is not shown:

grafik

The arrow is inserted in these code lines:

eval "git diff --name-status $commits -- ${files[*]} | sed -E 's/^([[:alnum:]]+)[[:space:]]+(.*)$/[\1]\t\2/'" |
    sed 's/\t/  ->  /2' | expand -t 8 |

The second sed command should replace a tab (which appears in the git output if files are renamed) with the -> string. Obviously this command does not the right thing in your environment. Can you verify this by giving me the output of the following two commands:

echo -e "M\ttestfile" | sed 's/\t/  ->  /2' 
echo -e "R100\ttestfile\tmyfile" | sed 's/\t/  ->  /2'

It should look like this:

grafik

keatsfonam commented 1 year ago

@carlfriedrich here is the output, sorry for the delay Screen Shot 2022-11-17 at 10 59 07 AM

This is on OSX 10.15.7 with zsh 5.7.1 (x86_64-apple-darwin19.0) sed doesn't have a version flag on OSX but here is an identifier: PROJECT:text_cmds-101.40.1

I also tested this on a linux box I have and the output looks like yours

carlfriedrich commented 1 year ago

@keatsfonam Thanks for your feedback. Unfortunately I don't have a Mac to verify this. Can you check if adding -E to sed changes anything?

echo -e "M\ttestfile" | sed -E 's/\t/  ->  /2' 
echo -e "R100\ttestfile\tmyfile" | sed -E 's/\t/  ->  /2'
keatsfonam commented 1 year ago

I think you mixed up the -E on your sed and echo. Neither fix the problem. However I just found that adding a $ in front seems to resolve the problem as far as I can tell on all my various systems ANSI-C Quoting https://www.gnu.org/software/bash/manual/html_node/ANSI_002dC-Quoting.html

Screen Shot 2022-11-18 at 1 01 36 PM

carlfriedrich commented 1 year ago

@keatsfonam Oh yes, I indeed mixed up the arguments in my example. Edited the post accordingly.

However, thanks for bringing up that solution! I will prepare a PR for that.

carlfriedrich commented 1 year ago

@keatsfonam I pushed PR #250 with a fix for this. Can you checkout the branch and verify that it fixes the issue for you?

keatsfonam commented 1 year ago

The issue still occurs in the line above adding the tab sed -E 's/^([[:alnum:]]+)[[:space:]]+(.*)$/[\1]\t\2/'"

Screen Shot 2022-11-30 at 11 53 52 AM

Using a $ to quote this one did not work (output is just []). I just got it working fine for me by just using a literal tab instead of a \t character. Not sure if this is a universal solution.

EDIT:bin/git-forgit b/bin/git-for
index c79a40e..8717251 100755
--- a/bin/git-forgit
+++ b/bin/git-forgit
@@ -123,7 +123,7 @@ _forgit_diff() {
         $FORGIT_DIFF_FZF_OPTS
         --prompt=\"$commits > \"
     "
-    eval "git diff --name-status $commits -- ${files[*]} | sed -E 's/^([[:alnum:]]+)[[:space:]]+(.*)$/[\1]\t\2/'" |
+    eval "git diff --name-status $commits -- ${files[*]} | sed -E 's/^([[:alnum:]]+)[[:space:]]+(.*)$/[\1] \2/'" |
         sed $'s/\t/  ->  /2' | expand -t 8 |
         FZF_DEFAULT_OPTS="$opts" fzf
     fzf_exit_code=$?

Screen Shot 2022-11-30 at 12 08 18 PM

carlfriedrich commented 1 year ago

@keatsfonam Thanks a lot for your feedback. I updated the branch in #250 with your proposed solution. Can you check again?

keatsfonam commented 1 year ago

@carlfriedrich Works great, thanks.