git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Stefan Beller <sbeller@google.com>
Cc: Junio C Hamano <gitster@pobox.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH 4/4] range-diff: indent special lines as context
Date: Tue, 14 Aug 2018 20:54:24 +0200 (DST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1808141713510.71@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <CAGZ79kYVw3AXLyB1fx07WojN3dLuxJLDrbWwC_7M=9aoB9YuCg@mail.gmail.com>

Hi Stefan,

On Mon, 13 Aug 2018, Stefan Beller wrote:

> > > 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.

> 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.

>   (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.

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.

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.

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.

Ciao,
Dscho

  reply	other threads:[~2018-08-14 18:54 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-10 22:49 [PATCH 0/4] Better colors in range-diff! Stefan Beller
2018-08-10 22:49 ` [PATCH 1/4] diff.c: emit_line_0 to take string instead of first sign Stefan Beller
2018-08-13 11:42   ` Johannes Schindelin
2018-08-13 18:19     ` Stefan Beller
2018-08-10 22:49 ` [PATCH 2/4] diff.c: add --output-indicator-{new, old, context} Stefan Beller
2018-08-13 11:47   ` Johannes Schindelin
2018-08-13 18:23     ` Stefan Beller
2018-08-10 22:49 ` [PATCH 3/4] range-diff: make use of different output indicators Stefan Beller
2018-08-13 11:51   ` Johannes Schindelin
2018-08-13 18:24     ` Stefan Beller
2018-08-10 22:49 ` [PATCH 4/4] range-diff: indent special lines as context Stefan Beller
2018-08-13 11:54   ` Johannes Schindelin
2018-08-13 18:36     ` Stefan Beller
2018-08-14 18:54       ` Johannes Schindelin [this message]
2018-08-14 19:05         ` Stefan Beller
2018-08-16  8:22           ` Johannes Schindelin
2018-08-17 20:43             ` [PATCH 0/3] Better colors in range-diff Stefan Beller
2018-08-17 20:43               ` [PATCH 1/3] diff.c: add --output-indicator-{new, old, context} Stefan Beller
2018-08-20 19:31                 ` Johannes Schindelin
2018-08-20 19:39                   ` Stefan Beller
2018-08-21 16:13                     ` Johannes Schindelin
2018-08-22 22:25                       ` [PATCH] diff.c: pass sign_index to emit_line_ws_markup Stefan Beller
2018-08-23 14:26                         ` Johannes Schindelin
2018-08-17 20:43               ` [PATCH 2/3] range-diff: make use of different output indicators Stefan Beller
2018-08-17 20:43               ` [PATCH 3/3] range-diff: indent special lines as context Stefan Beller
2018-08-17 22:04               ` [PATCH 0/3] Better colors in range-diff Junio C Hamano
2018-08-17 22:09                 ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1808141713510.71@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).