All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Palka <patrick@parcs.ath.cx>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks
Date: Thu, 18 Jun 2015 16:23:42 -0400	[thread overview]
Message-ID: <CA+C-WL9HmVVzwCou40ZFZQFyaETVpkoxQM5mpH2R0GfQPiwO7A@mail.gmail.com> (raw)
In-Reply-To: <xmqq1th8ga9b.fsf@gitster.dls.corp.google.com>

On Thu, Jun 18, 2015 at 2:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Patrick Palka <patrick@parcs.ath.cx> writes:
>
>>> I have this nagging feeling that it is just as likely that two
>>> uneven hunks align at the top as they align at the bottom, so while
>>> this might not hurt it may not be the right approach for a better
>>> solution, in the sense that when somebody really wants to do a
>>> better solution, this change and the original code may need to be
>>> ripped out and redone from scratch.
>>
>> Hmm, maybe. I stuck with assuming hunks are top-aligned because it
>> required less code to implement :)
>
> Yeah, I understand that.
>
> If we will need to rip out only this change but keep the original in
> order to implement a future better solution, then we might be better
> off not having this change (if we anticipate such a better solution
> to come reasonably soon), because it would make it more work for the
> final improved solution.  But if we need to rip out the original as
> well as this change while we do so, then having this patch would not
> make it more work, either.
>
> So as I said, I do not think it would hurt to have this as an
> incremental improvement (albeit going in a possibly wrong
> direction).
>
> Of course, it is a separate question if this change makes the output
> worse, by comparing unmatched early parts of two hunks and making
> nonsense highlight by calling highlight_pair() more often.  As long
> as that is not an issue, I am not opposed to this change, which was
> what I meant to say by "this might not hurt".
>

That makes sense.  The extra useless highlighting indeed is currently
a problem but it may yet be worked around.

  parent reply	other threads:[~2015-06-18 20:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-15 17:20 [PATCH] Improve contrib/diff-highlight to highlight unevenly-sized hunks Patrick Palka
2015-06-18 15:50 ` Junio C Hamano
2015-06-18 16:28   ` Patrick Palka
2015-06-18 18:08     ` Junio C Hamano
2015-06-18 19:04       ` Jeff King
2015-06-18 20:14         ` Patrick Palka
2015-06-18 20:45           ` Jeff King
2015-06-18 21:23             ` Jeff King
2015-06-18 21:39               ` Junio C Hamano
2015-06-18 22:25               ` Patrick Palka
2015-06-19  3:54               ` Jeff King
2015-06-19  4:49                 ` Junio C Hamano
2015-06-19  5:32                   ` Jeff King
2015-06-19  7:34                     ` Jeff King
2015-06-19 11:38                       ` Jeff King
2015-06-19 17:20                     ` Junio C Hamano
2015-06-18 23:06             ` Patrick Palka
2015-06-18 20:23       ` Patrick Palka [this message]
2015-06-18 19:08     ` Jeff King
2015-06-18 20:27       ` Patrick Palka

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=CA+C-WL9HmVVzwCou40ZFZQFyaETVpkoxQM5mpH2R0GfQPiwO7A@mail.gmail.com \
    --to=patrick@parcs.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.