Hello Junio
Thank you for your comment.
> 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).
To be more accurate, renaming output dose not always contains "{" or "}"
if there is no common part in source and destination paths, although
probably there are enough large possibility to include "{" or "}".
And, in the original patch, "{" or "}" is not kept, but changed to be kept
based Thomas Rast's feedback below.
(So, there was no possibility to have "{… => …}" in the original patch.)
On Oct 13, 2013, at 11:29 PM, Thomas Rast <[email protected]> wrote:
> Note that in the test, the generated line looks like this:
>
> {..._does_not_fit_in_a_single_line => .../path1 | 0
>
> I don't want to go all bikesheddey, but I think it's somewhat
> unfortunate that the elided parts do not correspond to each other. In
> particular, I think the closing brace should not be omitted. Perhaps
> something like this would be ideal (making it up on the spot, don't
> count characters):
>
> {...a_single_line => ..._as_the_first}/path1 | 0
And, it might be a bit nicer for me if the patch can be rejected(or ignored as
other patches)
from the beginning if the concept does not fit anyway.
# Though I know we can know more after seeing the implementation, anyway :-)
# And, my original explanation about the patch might be not enough.
Thanks !
---
Tsuneo Yoshioka (吉岡 恒夫)
[email protected]
On Oct 22, 2013, at 9:09 PM, Junio C Hamano <[email protected]> wrote:
> Yoshioka Tsuneo <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
