From: "René Scharfe" <l.s.r@web.de>
To: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>
Subject: Re: [Bug?] log -p -W showing the whole file for a patch that adds to the end?
Date: Sat, 28 May 2016 16:46:49 +0200 [thread overview]
Message-ID: <5749AF59.2070704@web.de> (raw)
In-Reply-To: <5740AC28.6010202@web.de>
Am 21.05.2016 um 20:42 schrieb René Scharfe:
> Am 11.05.2016 um 00:51 schrieb Junio C Hamano:
>> The helper function get_func_line() however gets confused when a
>> hunk adds a new function at the very end, and returns -1 to signal
>> that it did not find a suitable "function header line", i.e. the
>> beginning of previous function. The caller then takes this signal
>> and shows from the very beginning of the file. We end up showing
>> the entire file, starting from the very beginning to the end of the
>> newly added lines.
>
> In this case we need to look at the added lines in the post-image to
> see if the original context suffices. We currently only look at the
> pre-image. And if we have to extend the context simply search from the
> bottom of the pre-image. That's what the second patch does; the first
> one is just a small preparation.
>
> The last three patches introduce special handling of empty lines.
> Before them the code for -W only distinguished between function lines
> and non-function lines. Not allowing empty lines between functions to
> extend the context with -W is most useful in the case of appended full
> functions -- otherwise we'd get the preceding function shown as well as
> it "belongs" to the empty line separating them.
>
> And if we do that then it's easy and more consistent to stop showing
> empty lines trailing functions with -W (unless they're already included
> in the one requested with -u/-U, of course). Doing the same for grep
> then is only fair.
>
> Considering empty lines as uninteresting collides with languages like
> Whitespace. I'm not sure -W was useful for it to begin with, though.
> Any other possible downsides?
>
> NB: Comments are still not handled specially. That means they are
> considered to be part of the preceding function. Fixing that is not in
> scope for this series. (And I'm not sure it would be worth the hassle.)
>
> diff: factor out match_func_rec()
> diff: handle appended chunks better with -W
> diff: ignore empty lines before added functions with -W
> diff: don't include common trailing empty lines with -W
> grep: don't extend context to trailing empty lines with -W
>
> grep.c | 28 ++++++++++++++++++++++++--
> xdiff/xemit.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 82 insertions(+), 9 deletions(-)
And here's the second round, this time with tests, a fix for a
signedness issue found by Ramsay, a small cleanup of the new function
is_empty_rec() and a new fix for the case of using git diff -W -U0.
An interdiff for the files touched by the original series is at the
bottom.
t4051: rewrite, add more tests
xdiff: factor out match_func_rec()
xdiff: handle appended chunks better with -W
xdiff: ignore empty lines before added functions with -W
xdiff: -W: don't include common trailing empty lines in context
xdiff: don't trim common tail with -W
t7810: add test for grep -W and trailing empty context lines
grep: -W: don't extend context to trailing empty lines
grep.c | 28 ++++-
t/t4051-diff-function-context.sh | 216 +++++++++++++++++++++++++--------------
t/t4051/appended1.c | 15 +++
t/t4051/appended2.c | 35 +++++++
t/t4051/dummy.c | 7 ++
t/t4051/hello.c | 21 ++++
t/t4051/includes.c | 20 ++++
t/t7810-grep.sh | 19 +++-
xdiff-interface.c | 10 +-
xdiff/xemit.c | 62 +++++++++--
10 files changed, 341 insertions(+), 92 deletions(-)
create mode 100644 t/t4051/appended1.c
create mode 100644 t/t4051/appended2.c
create mode 100644 t/t4051/dummy.c
create mode 100644 t/t4051/hello.c
create mode 100644 t/t4051/includes.c
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index d0c0738..bfa53d3 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -155,12 +155,12 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
return -1;
}
-static int is_empty_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri)
+static int is_empty_rec(xdfile_t *xdf, long ri)
{
const char *rec;
long len = xdl_get_rec(xdf, ri, &rec);
- while (len > 0 && isspace(*rec)) {
+ while (len > 0 && XDL_ISSPACE(*rec)) {
rec++;
len--;
}
@@ -196,7 +196,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
* starting with empty lines.
*/
while (i2 < xe->xdf2.nrec &&
- is_empty_rec(&xe->xdf2, xecfg, i2))
+ is_empty_rec(&xe->xdf2, i2))
i2++;
if (i2 < xe->xdf2.nrec &&
match_func_rec(&xe->xdf2, xecfg, i2,
@@ -231,8 +231,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
long fe1 = get_func_line(xe, xecfg, NULL,
xche->i1 + xche->chg1,
xe->xdf1.nrec);
- while (fe1 > 0 &&
- is_empty_rec(&xe->xdf1, xecfg, fe1 - 1))
+ while (fe1 > 0 && is_empty_rec(&xe->xdf1, fe1 - 1))
fe1--;
if (fe1 < 0)
fe1 = xe->xdf1.nrec;
next prev parent reply other threads:[~2016-05-28 14:52 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-10 21:51 [Bug?] log -p -W showing the whole file for a patch that adds to the end? Junio C Hamano
2016-05-10 22:51 ` Junio C Hamano
2016-05-12 22:12 ` René Scharfe
2016-05-21 18:42 ` René Scharfe
2016-05-21 18:45 ` [PATCH 1/5] diff: factor out match_func_rec() René Scharfe
2016-05-21 18:45 ` [PATCH 2/5] diff: handle appended chunks better with -W René Scharfe
2016-05-21 18:46 ` [PATCH 3/5] diff: ignore empty lines before added functions " René Scharfe
2016-05-21 18:46 ` [PATCH 4/5] diff: don't include common trailing empty lines " René Scharfe
2016-05-21 18:47 ` [PATCH 5/5] grep: don't extend context to " René Scharfe
2016-05-24 18:16 ` [Bug?] log -p -W showing the whole file for a patch that adds to the end? Junio C Hamano
2016-05-26 8:54 ` René Scharfe
2016-05-26 17:05 ` Junio C Hamano
2016-05-27 17:13 ` René Scharfe
2016-05-28 14:46 ` René Scharfe [this message]
2016-05-28 14:57 ` [PATCH v2 1/8] t4051: rewrite, add more tests René Scharfe
2016-05-29 23:55 ` Junio C Hamano
2016-05-30 20:55 ` René Scharfe
2016-05-31 20:00 ` [PATCH v2.5 " René Scharfe
2016-05-31 20:15 ` René Scharfe
2016-05-31 21:23 ` Junio C Hamano
2016-05-28 14:58 ` [PATCH v2 2/8] xdiff: factor out match_func_rec() René Scharfe
2016-05-28 15:00 ` [PATCH v2 3/8] xdiff: handle appended chunks better with -W René Scharfe
2016-05-28 15:02 ` [PATCH v2 4/8] xdiff: ignore empty lines before added functions " René Scharfe
2016-05-28 15:03 ` [PATCH v2 5/8] xdiff: -W: don't include common trailing empty lines in context René Scharfe
2016-05-28 15:04 ` [PATCH v2 6/8] xdiff: don't trim common tail with -W René Scharfe
2016-05-28 15:05 ` [PATCH v2 7/8] t7810: add test for grep -W and trailing empty context lines René Scharfe
2016-05-28 15:06 ` [PATCH v2 8/8] grep: -W: don't extend context to trailing empty lines René Scharfe
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5749AF59.2070704@web.de \
--to=l.s.r@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=ramsay@ramsayjones.plus.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).