From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Git Mailing List <git@vger.kernel.org>,
Davide Libenzi <davidel@xmailserver.org>
Subject: Re: [PATCH, RFC] diff: add option to show context between close chunks
Date: Mon, 20 Oct 2008 20:06:25 +0200 [thread overview]
Message-ID: <48FCC8A1.5090109@lsrfire.ath.cx> (raw)
In-Reply-To: <48FC9685.8030704@viscovery.net>
Johannes Sixt schrieb:
> Why can't you just use -U6 instead instead of --inter-chunk-context=3? If
> this is intended for human consumption anyway, then you can just as well
> increase the overall number of context lines: You get extra context lines
> in the places where hunks are not fused, but this cannot be a disadvantage
> for the targeted audience.
The extra lines _are_ a disadvantage for me, since each chunk gets them
while only close ones need them -- that's wasted space. The intent is
to save time by not having to apply and rediff a patch on a mailing list
in order to see the hidden text; part of the savings would be lost if
the reader had to skip over extra lines.
Besides, this option is probably most useful if set by default on
machines not controlled by me. E.g. I wouldn't want to change an option
on a gitweb page, I'd want to get fused chunks by default. I wouldn't
want to write "best viewed with -U6" in a patch description, I'd want it
to just work (e.g. for gitk users).
I have to admit my main motivation was that one line gap, where a chunk
header hid an interesting line of context. Showing it didn't change the
length of the patch, so I found this to be a sad wastage.
Optimizing patches for readability makes sense since they are such an
important part of our communication here. I think --i-c-c=1 results in
an obvious win without much of a downside. I was surprised to find that
almost 4% of the chunks in git 1.6.0 would be eliminated by that option:
$ git log -p v1.6.0 | grep -c '^@@ '
60544
$ git log -p v1.6.0 --inter-chunk-context=1 | grep -c '^@@ '
58471
But perhaps a higher value would be even better?
$ git log v1.6.0 | wc -l
225441
$ git log -p v1.6.0 | wc -l
1736188
$ git log -p --inter-chunk-context=10 v1.6.0 | wc -l
1779048
$ git log -p -U8 v1.6.0 | wc -l
2214448
Well, I don't know if those patches are easier to read (haven't looked
at them), but I imagine that some related changes are presented with the
full context between them (e.g. changes to loop header and footer,
function signature and body). The numbers only show that it's
affordable (3% more lines with -i-c-c=10, 30% more lines with the
comparable -U8).
(Why 10? With the default of -U3, a chunk is 6 lines of context plus at
least one line of actual change. Two chunks are at least 14 lines long,
thus only 10 more fit into 24 lines, i.e. a terminal window..)
René
next prev parent reply other threads:[~2008-10-20 18:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-19 17:59 [PATCH, RFC] diff: add option to show context between close chunks René Scharfe
2008-10-20 14:32 ` Johannes Sixt
2008-10-20 18:06 ` René Scharfe [this message]
2008-10-21 6:09 ` Johannes Sixt
2008-10-21 20:45 ` René Scharfe
2008-10-20 23:43 ` Junio C Hamano
2008-10-21 6:35 ` Johannes Sixt
2008-10-21 7:12 ` Junio C Hamano
2008-10-21 11:20 ` Jeff King
2008-10-21 20:48 ` René Scharfe
2008-10-21 18:16 ` Daniel Barkalow
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=48FCC8A1.5090109@lsrfire.ath.cx \
--to=rene.scharfe@lsrfire.ath.cx \
--cc=davidel@xmailserver.org \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.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 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).