Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible

Hello Thomas

Can you briefly describe what you changed in v7 and v8, both compared to
earlier versions and between v7 and v8?

On v7, <sfx>'s basename part is tried to kept. On v7, whole <sfx> part is tried to kept.
For example, in case below:
   parent_path{sourceDirectory => DestinationDirectory}path1/path2//longlongFilename.txt
 On v7, this can be like:
   …{...ceDirectory => …onDirectory}.../longlongFilename.txt
On v8, it will be like:
   …{...irectory => …irectory}path1/path2/longlongFilename.txt


This change is based on the review from Junio below.
(I myself is not sure what is the better way.)
================================
On Oct 17, 2013, at 10:29 PM, Junio C Hamano [off-list ref] wrote:

I am not sure if distributing the burden of truncation equally to
three parts so that the resulting pieces are of similar lengths is
really a good idea.  Between these two

	{...SourceDirectory => ...nationDirectory}...ileThatWasMoved 
	{...ceDirectory => ...ionDirectory}nameOfTheFileThatWasMoved

that attempt to show that the file nameOfTheFileThatWasMoved was
moved from the longSourceDirectory to the DestinationDirectory, the
latter is much more informative, I would think.

On Oct 18, 2013, at 1:38 AM, Junio C Hamano [off-list ref] wrote:

Yoshioka Tsuneo [off-list ref] writes:

quoted

In the "[PATCH v7]", I changed to keep filename part of suffix to handle
above case, but not always keep directory part because I feel totally
keeping all part of long suffix including directory name may cause output like:
   …{… => …}…ongPath1/LongPath2/nameOfTheFileThatWasMoved 
And, above may be worse than:
  ...{...ceDirectory => …ionDirectory}.../nameOfTheFileThatWasMoved
I think.

I am not sure if I agree.

Losing LongPath2 part may be more significant data loss than losing
a single bit that says the change is a rename, as the latter may not
quite tell us what these two directories were anyway.

================================

Also, I guess Junio might be suspicious to the idea to keep arrow("=>") itself, maybe ?
=================================
(From What's cooking in git.git (Oct 2013, #04; Fri, 18))
- diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible

Attempts to give more weight on the fact that a filepair represents
a rename than showing substring of the actual path when diffstat
lines are not wide enough.

I am not sure if that is solving a right problem, though.
=================================

Thanks!

---
Tsuneo Yoshioka (吉岡 恒夫)
yoshiokatsuneo@gmail.com




On Oct 19, 2013, at 9:24 AM, Thomas Rast [off-list ref] wrote:

Yoshioka Tsuneo [off-list ref] writes:

quoted

"git diff -M --stat" can detect rename and show renamed file name like
"foofoofoo => barbarbar".

Before this commit, this output is shortened always by omitting left most
part like "...foo => barbarbar". So, if the destination filename is too long,
source filename putting left or arrow can be totally omitted like
"...barbarbar", without including any of "foofoofoo =>".
In such a case where arrow symbol is omitted, there is no way to know
whether the file is renamed or existed in the original.

Make sure there is always an arrow, like "...foo => ...bar".

The output can contain curly braces('{','}') for grouping.
So, in general, the output format is "<pfx>{<mid_a> => <mid_b>}<sfx>"

To keep arrow("=>"), try to omit <pfx> as long as possible at first
because later part or changing part will be the more important part.
If it is not enough, shorten <mid_a>, <mid_b> trying to have the same
maximum length.
If it is not enough yet, omit <sfx>.

Signed-off-by: Tsuneo Yoshioka <redacted>
Test-added-by: Thomas Rast [off-list ref]
---

Can you briefly describe what you changed in v7 and v8, both compared to
earlier versions and between v7 and v8?

It would be very nice if you could always include such a "patch
changelog" after the "---" above.  git-am will ignore the text between
"---" and the diff, so you can write comments for the reviewers there
without creating noise in the commit message.

Also, please keep reviewers in the Cc list for future discussion/patches
so that they will see them.

-- 
Thomas Rast
tr@thomasrast.ch