Re: Bug report: "Use of uninitialized value $_ in print"

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

 



On 02/03/2018, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jeff King <peff@xxxxxxxx> writes:
>> Unfortunately, I don't think there's an easy way to do what you want
>> (show word-diffs but apply the full diff).
>
> The current "word-diff" discards the information on where the lines
> end, and it is pretty much hopeless/useless in the context of "add
> -p".

This is unfortunate.

I am familiar with the model-view-controller ("MVC") paradigm used in
some software packages. I had hoped that Git followed this paradigm
somewhat, and cleanly separated the underlying model of the diff (i.e.
a representation that would be generated in all cases where a diff is
needed), and the view of the diff (i.e. the visual representation,
e.g. word-diff, line diff, colored, non-colored, etc, as requested by
the user).

Such separation of concerns strikes me as being the best approach
here. It could be a lot of work, but I would be grateful if the Git
developers/maintainers could work towards it as an end goal for this
aspect of Git's architecture.

Unfortunately, I am not very familiar with the Git codebase, nor
well-versed in its primary languages, so I can't be of much help here
:(

> It would be a good addition to revamp it so that it keeps the
> original lines in pre/post images but only colors/highlights the
> byte ranges in such a way that (1) on a pre-image line, paint only
> the byte range that do not appear in the post-image, and (2) on a
> post-image line, paint only the byte range that do not appear in the
> pre-image.
>
> E.g. Given these two
>
>     diff --git a/1 b/2
>     index aa49234b68..1cd9f6b5fd 100644
>     --- a/1
>     +++ b/2
>     @@ -1,2 +1 @@
>     -a b c e
>     -f g i j
>     +a b d f g h
>
> the current word-diff would give (I cannot do colors here, so I'll
> upcase the colored letters):
>
>     @@ -1,2 +1 @@
>     a b C ED f g I JH
>
> as the output, but if we produced
>
>     @@ -1,2 +1 @@
>     -a b C E
>     -f g I J
>     +a b D f g H
>
> instead, then colored "add -p" would become easier to see.
>
> And that would be useful outside the context of "add -p", which is a
> huge plus.

This would be much better than the current situation, where the visual
representation gives misleading feedback about the underlying diff,
and where the error I reported crops up.

However, in my opinion your proposed solution would still be not as
good as separating the two concerns, as described earlier in this
email, on two fronts:

1. It would yield, IIUC, less flexibility to create new kinds of view
based on a consistent, standardised underlying model.

2. It is harder to read, for some types of input (e.g. prose) than the
view generated by the existing word-diff algorithm. Your approach, as
outlined in your example above, requires the reader to visually jump
(saccade) between two lines that are separated by an intervening line,
in order to see what has changed. The existing word-diff is much
easier to use: it puts the changes right next to each other, avoiding
line-skipping and allowing horizontal saccades (which are much more
familiar to people used to languages written in left-to-right or
right-to-left scripts).

I don't want to sound negative. As I say, I think your solution is a
big improvement on what currently exists. But I would see it as an
intermediate solution - a stopgap - rather than an endpoint of
development.

In other words, if your solution would be much quicker to implement
than the one I proposed, then please go ahead with yours first, and
please add mine to the longer-term to-do list :)

Thank you again for helping to make Git such a great tool, and for
working tirelessly to improve it further!

P.S. There is a related StackOverflow question here:
https://stackoverflow.com/questions/49058817/git-add-patch-with-word-diff



[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]

  Powered by Linux