Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 rename and copy operations produce empty output #1548

Open
RuRo opened this issue Oct 1, 2023 · 9 comments
Open

🐛 rename and copy operations produce empty output #1548

RuRo opened this issue Oct 1, 2023 · 9 comments

Comments

@RuRo
Copy link

RuRo commented Oct 1, 2023

Running this reproduction script:

#!/usr/bin/env bash

cd "$(mktemp -d)"

export GIT_CONFIG_GLOBAL=/dev/null
export GIT_CONFIG_NOSYSTEM=1
export PAGER=cat

git -c init.defaultBranch=master init . >/dev/null
git config user.email "[email protected]"
git config user.name "Best Tester"

echo -e "something\nblah\nblah\nlong\nfile" > here
echo -e "different\nlorem\nipsum\ndolor\nsit\namet" > there
git add .
git commit -m "initial commit" >/dev/null

mv here moved
cp there copied
git add .
git commit -m "second commit" >/dev/null

echo "==== regular diff ===="
git --no-pager          diff -C -C HEAD~ HEAD
echo "======================"

echo

echo "==== $(delta --version | tr -d '\n') ===="
git -c core.pager=delta diff -C -C HEAD~ HEAD
echo "======================"

Produces the following output:

==== regular diff ====
diff --git a/there b/copied
similarity index 100%
copy from there
copy to copied
diff --git a/here b/moved
similarity index 100%
rename from here
rename to moved
======================

==== delta 0.16.5 ====
======================
@RuRo
Copy link
Author

RuRo commented Oct 1, 2023

This looks like it might be a regression from #392

@imbrish
Copy link
Contributor

imbrish commented Mar 4, 2024

Hey! Piping the diff you provided produces correct output in current main. You may want to try the same?

image

@RuRo
Copy link
Author

RuRo commented Mar 4, 2024

Okay, so this is a separate issue, but it seems that delta doesn't respect the GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM environment variables which is all kinds of wrong. So my "minimal" reproduction didn't work because delta was still reading my gitconfig.


After figuring that out, I was finally able to find the problematic option: file-style = omit. You could argue that this is working as intended, however this behaviour doesn't really make sense. My understanding is that the main reason for the existence of file-style = omit is to complement hunk-header-style = file line-number, but since renames and copies don't produce hunks, then it doesn't make sense to completely omit this information.

So IMHO, file-style = omit should only omit "empty" file section headers and keep the rename/copy information. Or at the very least there should be an option that does that.

Or alternatively, rename/copy operations should be represented by their own pseudo-hunks when file-style is set to omit.

@dandavison
Copy link
Owner

it seems that delta doesn't respect the GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM environment variables

What delta version? I thought that this was a libgit2 issue that had been solved.

@RuRo
Copy link
Author

RuRo commented Mar 4, 2024

I patched the nixpkgs derivation of delta to compile from dcae5bc (main at the time of writing). Though delta --version itself still reports the version as 0.16.5.

P.S. looking at the build logs, I can see that libgit2-sys v0.15.2+1.6.4 was compiled/used by cargo.

P.P.S. looking at the upstream libgit2 repo, it seems that the relevant PR libgit2/libgit2#6544 was first released in version 1.7.0.

dandavison added a commit that referenced this issue Mar 4, 2024
dandavison added a commit that referenced this issue Mar 4, 2024
@dandavison dandavison mentioned this issue Mar 4, 2024
dandavison added a commit that referenced this issue Mar 4, 2024
@dandavison
Copy link
Owner

Thanks @RuRo! I update git2 in main: #1647

@RuRo
Copy link
Author

RuRo commented Mar 5, 2024

Great. Circling back to the original issue, what do you think about the current behaviour of file-style = omit + hunk-header-style = file line-number for renames/copies?

@imbrish
Copy link
Contributor

imbrish commented Mar 5, 2024

For diffs of binary files, I was thinking about keeping the line Binary files a/foo and b/bar differ. Similarly standalone diff can report Files foo and bar are identical. Such line is not present for renames and copies of git diff, but I think it may be useful to have it.

@dandavison what do you think about adding such line for otherwise empty diffs? It could be hidden behind an option like --show-status-line that would be disabled by default, and perhaps enabled automatically along with file-style = omit.

@RuRo would something like that work in your case?

@RuRo
Copy link
Author

RuRo commented Mar 5, 2024

@RuRo would something like that work in your case?

I think so. I guess, the specifics could use some bikeshedding, but I'd generally be okay with any solution that could show this information without showing the redundant (when used with hunk-header-style = file line-number) file "section" headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants