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]

 



Yoshioka Tsuneo <yoshiokatsuneo@xxxxxxxxx> writes:

> Also, I guess Junio might be suspicious to the idea to keep arrow("=>") itself, maybe ?

I think there is no single "right" solution to this issue, and it
has to boils down to the taste.

When you are viewing "diff --stat -M" output in wide-enough medium,
you are seeing three pieces of information: what the source path
was, what the destination path will be, and what amount of change is
made with the change. When the output width is too narrow to show
these paths, with the current code, you see truncated destination
path, possibly without the source path, but this patch will show the
source and the destination paths, both of which are truncated even
more severely, because it always has to spend display columns for an
extra "..." (to show truncation of the source side), " => " (to show
that it is a rename), and <"{","}"> pair (again to show that it is a
rename).  If the destination does not fit, the output before this
patch would have thrown these away as part of left-truncation, to
show the destination path as maximally as possible.  We do not have
even half the width of the current "truncated to be destination
only" output for each path.

I am afraid that in the cases where the patch makes a difference,
what happens would be that you can no longer tell what source or
destination paths really are, because the leading directory part
gets truncated too much, and if we didn't have this patch, at least
you can tell what destination path is affected.  We would trade the
guessability of at least one path (the destination) with just a
single bit of information (an unidentifiable path got renamed to
another unidentifiable path).

I am not yet convinced that it is a good trade-off.  Especially
given the diffstat output is not about files but more about
contents, between an output in the extreme case the version after
the patch needs to produce

	{... => ...}/controller/Makefile | 7 +++++++

that tells us "7 lines were updated in the procedure to build some
unknown controller by copying or renaming from the build procedure
of some other unknown controller", and the output the current code
would give to the same rename

	.}/fooGadget/controller/Makefile | 7 +++++++
        
that tells us "7 lines were updated in the build procedure for the
foo Gadget", I think the latter contains more useful information,
even though it does lose one bit of information ("there was a rename
involved in producing this final path") compared to the version with
the patch.

So you are correct to say that I am still skeptical.

In any case, the output from "diff --stat -M" should match the
output from "apply --stat -M", I think.
--
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]