Thread (25 messages) 25 messages, 2 authors, 2018-08-23

Re: [PATCH 4/4] range-diff: indent special lines as context

flat view

From: Stefan Beller <hidden>
Date: 2018-08-14 19:05:51

On Tue, Aug 14, 2018 at 11:54 AM Johannes Schindelin
[off-list ref] wrote:
Hi Stefan,

On Mon, 13 Aug 2018, Stefan Beller wrote:
quoted
quoted
quoted
The later lines that indicate a change to the Makefile will be treated as
context both in the outer and inner diff, such that those lines stay
regular color.
While I am a fan of having those lines colored correctly, I have to admit
that I am not exactly enthusiastic about that extra indentation...

Otherwise, this looks good to me.
Can you explain what makes you less enthused about the indentation?

Advantage:
* allows easy coloring (easy implementation)
Disadvantage:
* formats change,
This is it. It breaks my visual flow.
quoted
but the range diff is still in its early design phase, so we're not
breaking things, yet?
Indeed. We're not breaking things. If you feel strongly about it, we can
have that indentation, I *can* get used to it.
I only feel strongly about it now as that is the *easiest* way to make
the colors
look like I want them to look. And I really value colors in the range-diff.
Earlier you said that color-less range-diff is nearly useless for you and I
thought it was hyperbole, but by now I realize how much truth you spoke.
So getting the colors fixed to not markup files (+++/ --- lines of the inner
diff) is a high priority for me. So high that I would compromise on the
indentation/flow of these corner case areas.
quoted
  (Do we ever plan on sending range-diff patches that can be applied to
  rewrite history? I am very uncertain on such a feature request.  It
  sounds cool, though)
I remember that I heard you discussing this internally. I am not too big a
fan of this idea, I have to admit. The range diff seems more designed to
explain how a patch series evolved, rather than providing machine-readable
data that allows to recreate said evolution. For example, the committer
information as well as the date are missing, which would preclude a
faithful reconstruction.
Ah! good point. Though we could just work around that and use the email
date for the new author dates. ;-)
And that is not all: if you wanted to "apply" a range diff, you would need
to know more about the base(s) of the two commit ranges. You would need to
know that they are at least very similar to the base onto which you want
to apply this.
You would say so in the cover letter "This is a resend of sb/range-diff-colors"
and by the knowledge of that tip only and the range-diff you would
know how the new series would look like, even if it was rebased.
And quite seriously, this would be the wrong way to go in my mind. We have
a very efficient data format to transport all of that information: the Git
bundle.
The bundle format is very efficient for machine transport, but I thought the
whole point of the mailing list was easy human readable parts, i.e. you can
point out things in a diff, which you could also do in a range-diff to some
extend. We would loose some of the "fresh eyes" as you'd only see the
changed part of the series. :-/ So yeah even for the workflow this seems
a net-negative. I just thought it would be cool.
Let's not overload the range diff format with multiple, partially
contradicting purposes. Think "separation of concerns". It's the same
issue, really, as trying to send highly structured data such as bug
reports or code contributions via a medium meant to send unstructured
plain or formatted text back and forth between human beings.
ok.

Thanks,
Stefan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help