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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <gitster@xxxxxxxxx> 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 <gitster@xxxxxxxxx> wrote:
> Yoshioka Tsuneo <yoshiokatsuneo@xxxxxxxxx> writes:
> 
>> 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@xxxxxxxxx




On Oct 19, 2013, at 9:24 AM, Thomas Rast <tr@xxxxxxxxxxxxx> wrote:

> Yoshioka Tsuneo <yoshiokatsuneo@xxxxxxxxx> writes:
> 
>> "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 <yoshiokatsuneo@xxxxxxxxx>
>> Test-added-by: Thomas Rast <trast@xxxxxxxxxxx>
>> ---
> 
> 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@xxxxxxxxxxxxx

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]