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

Improve forgit diff #189

Merged
merged 3 commits into from
Mar 21, 2022
Merged

Improve forgit diff #189

merged 3 commits into from
Mar 21, 2022

Conversation

carlfriedrich
Copy link
Collaborator

@carlfriedrich carlfriedrich commented Mar 11, 2022

Check list

  • I have performed a self-review of my code
  • [n/a] I have commented my code in hard-to-understand areas
  • [n/a] I have made corresponding changes to the documentation

Description

Fix two bugs in forgit::diff:

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation change

Test environment

  • Shell
    • bash
    • zsh
    • fish
  • OS
    • Linux
    • Mac OS X
    • Windows
    • Others:

For "git diff" to display a move/rename action correctly, it has to be
passed both the old and the new file name. Change the gd preview command
accordingly: instead of constructing a path out of the repo directory
and the file name (which would not work with multiple file names), cd to
the repo directory and use the file names as direct arguments to "git
diff" instead.

Change the regex used to parse the "git diff --name-status" output so
that it supports the "Rxxx" syntax for moved/renamed files.

Use tabs instead of hardcoded number of spaces along with "expand" to
have file names horizontally aligned.

Fixes wfxr#188
Show an arrow instead of just blank spaces when a file has been moved
or renamed:

[R100]  foo.txt  ->  bar.txt

The arrow is added as a replacement for the second tabspace of the "git
diff --name-status" command. For the preview command it is replaced by a
single space again.
@carlfriedrich carlfriedrich changed the title Add support for move/rename in 'gdImprove forgit diff Improve forgit diff Mar 11, 2022
@cjappl cjappl requested a review from wfxr March 19, 2022 21:54
@wfxr
Copy link
Owner

wfxr commented Mar 21, 2022

LGTM! @carlfriedrich Thanks for your contribution!

@wfxr wfxr merged commit fca195a into wfxr:master Mar 21, 2022
@carlfriedrich carlfriedrich deleted the improve-forgit-diff branch March 21, 2022 08:59
@cjappl
Copy link
Collaborator

cjappl commented Mar 21, 2022

@carlfriedrich Will you post an example with screenshots of how #188 is supposed to work? Trying to move all this functionality over to fish, and I can't seem to get it to work.

What I'm looking for is something like:

git mv README.md README2.md
gd

and posting some screenshots of how it looks on your side? Just want a nice reproducible target to aim for!

1 similar comment
@cjappl
Copy link
Collaborator

cjappl commented Mar 21, 2022

@carlfriedrich Will you post an example with screenshots of how #188 is supposed to work? Trying to move all this functionality over to fish, and I can't seem to get it to work.

What I'm looking for is something like:

git mv README.md README2.md
gd

and posting some screenshots of how it looks on your side? Just want a nice reproducible target to aim for!

@cjappl cjappl mentioned this pull request Mar 21, 2022
15 tasks
cjappl added a commit that referenced this pull request Mar 21, 2022
Checked in a bunch of things that were only for bash #189 #186 #185 #190 #188
@carlfriedrich
Copy link
Collaborator Author

@cjappl Sure, see the minimal example from the the description of #188:

mkdir myrepo
cd myrepo
git init
echo "wow" > foo.txt
git add foo.txt
git commit -m "Commit 1"
git mv foo.txt bar.txt
echo "oh" > baz.txt
git add baz.txt

Calling gd --staged on this repo showed the following before:

grafik

Notice the empty preview window and the wrongly displayed first line of the file list (state not in brackets, two filenames without context).

With #188 merged the preview window works again with the diff showing the rename (this is with diff-so-fancy installed) and the file list displays the corresponding entry correctly:

grafik

@cjappl
Copy link
Collaborator

cjappl commented Mar 21, 2022

Beautiful!! Thanks for your contribution and this description, very helpful!!

@cjappl cjappl self-assigned this Mar 21, 2022
carlfriedrich added a commit to carlfriedrich/forgit that referenced this pull request Jan 30, 2023
This had been fixed before in wfxr#189, but obviously the patch broke
support for whitespaces in file names. That was fixed in wfxr#204, which
in turn broke support for renames again.

Unfortunately we cannot support both cases together with the existing
'xargs' implementation, because the arguments are interpreted either
as one single file name (which breaks renames, because we have two files
passed in this case) or as separate file names (which breaks whitespace
support, because git would interpret each word of the file name as a
separate file).

This patch moves away from xargs and stores the file names in a bash
array instead. This makes it possible to pass them to "git diff" quoted
one by one using the "${array[@]}" notation.
carlfriedrich added a commit that referenced this pull request Feb 28, 2023
This had been fixed before in #189, but obviously the patch broke
support for whitespaces in file names. That was fixed in #204, which
in turn broke support for renames again.

Prepare the list of file names with the null-character \0 as a delimiter,
so that we can use "xargs -0" to read it. This makes renames as well
as filenames with spaces work correctly.
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

Successfully merging this pull request may close these issues.

None yet

3 participants