git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christoph Mallon <christoph.mallon@gmx.de>
Cc: Jonathan Nieder <jrnieder@gmail.com>, git@vger.kernel.org
Subject: Re: git diff --check always shows line 1 for blank at EOF for new files
Date: Sun, 10 Oct 2010 03:46:27 -0700	[thread overview]
Message-ID: <7vpqvitjek.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4CADBE01.4000901@gmx.de> (Christoph Mallon's message of "Thu\, 07 Oct 2010 14\:33\:05 +0200")

Christoph Mallon <christoph.mallon@gmx.de> writes:

> On 07.10.2010 11:32, Jonathan Nieder wrote:
>> Try this:
>>
>>      git init test
>>      cd test
>>      printf 'a\nb\nc\n'>  file
>>      git add -A
>>      echo hello>>  file
>>      echo>>  file
>>      git diff --check
>>
>> As you can see, it still returns line 4, which is the beginning of the
>> hunk that adds the blank line.
>>
>> A change to make it print the line number of the blank line itself
>> does not sound so bad, though.
>
> You're right. I think, the attached patch corrects the issue.
>
> From 45eaa882dae35f2976e77cf0b6b06be78283c13f Mon Sep 17 00:00:00 2001
> From: Christoph Mallon <christoph.mallon@gmx.de>
> Date: Thu, 7 Oct 2010 14:22:02 +0200
> Subject: [PATCH] When git diff --check detects a blank line at EOF, show the line number of the empty line, not the line number of the beginning of the hunk.
>
> ---

Could you check Documentation/SubmittingPatches, please?

>  diff.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 71efa8e..452fdf4 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2153,7 +2153,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
>  
>  			ecbdata.ws_rule = data.ws_rule;
>  			check_blank_at_eof(&mf1, &mf2, &ecbdata);
> -			blank_at_eof = ecbdata.blank_at_eof_in_preimage;
> +			blank_at_eof = ecbdata.blank_at_eof_in_postimage;

Hmm, I am confused.  Your subject line claims that for this patch:

        diff --git a/Makefile b/Makefile
        index 1f1ce04..90075e0 100644
        --- a/Makefile
        +++ b/Makefile
        @@ -2375,3 +2375,8 @@ cover_db: coverage-report

         cover_db_html: cover_db
                cover -report html -outputdir cover_db_html cover_db
        +
        +new-rule:
        +	echo here is a new rule
        +
        +

the code used to diagnose the error by showing the first line of the hunk
(2375 taken from either -2375 or +2375, it does not matter much).  The
claim is that your patch makes it to show the first line in the trailing
blank line block (the line after "here is a new rule", i.e. line 2381 in
the postimage).

But my reading of the code is that it is showing (incorrectly) the first
line of the trailing blank line block in the preimage (in this case,
because our codebase is clean, there is zero line of trailing blank line
block in the preimage, so it points at 2378.

So the patch looks correct, but the diagnosis / description seems wrong.
Since I am asking you to refer to SubmittingPatches to give a better
subject line, patch description and a sign-off anyway,...

Thanks.

      reply	other threads:[~2010-10-10 10:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-06  9:27 git diff --check always shows line 1 for blank at EOF for new files Christoph Mallon
2010-10-07  9:32 ` Jonathan Nieder
2010-10-07 12:33   ` Christoph Mallon
2010-10-10 10:46     ` Junio C Hamano [this message]

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=7vpqvitjek.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=christoph.mallon@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.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).