From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Kristian Høgsberg" <krh@redhat.com>,
gitster@pobox.com, git@vger.kernel.org
Subject: Re: [PATCH v2] builtin-commit: Include the diff in the commit message when verbose.
Date: Thu, 22 Nov 2007 11:14:29 -0800 [thread overview]
Message-ID: <7vzlx63xey.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0711221049350.27959@racer.site> (Johannes Schindelin's message of "Thu, 22 Nov 2007 10:52:04 +0000 (GMT)")
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Wed, 21 Nov 2007, Kristian Høgsberg wrote:
>
>> +
>> + /* Truncate the message just before the diff, if any. */
>> + p = strstr(sb.buf, "\ndiff --git a/");
>> + if (p != NULL)
>> + strbuf_setlen(&sb, p - sb.buf);
>> +
>
> Is this related to the change in wt_status? If so, wouldn't we want to
> suppress the diff, instead of generating it, and then killing it later?
This corresponds to the sed script near l.545 in git-commit.sh.
I've been wondering if it would be better not to have this logic
but instead "git commit -v" to show the diff text prefixed with
'# ' to make it a proper comment, by the way.
> Besides, you'd want to leave the \n there: strbuf_setlen(&sb, p + 1 -
> sb.buf);
Yup.
>> + /* Sigh, the entire diff machinery is hardcoded to output to
>> + * stdout. Do the dup-dance...*/
>
> I wonder how much effort it would be to change that. Not that it would
> help too much, since we want the output in a strbuf anyway.
The codepath is preparing COMMIT_EDITMSG file for the user to
edit (or to the standard output, if this is "git status -v").
I do not know about "strbuf" part; resulting patch text could be
large and in most cases the callers would rather flush it to
outside (either pipe or file) as soon as possible than having to
keep it all in core.
I was trying to do the stdout -> FILE *fp conversion of diff.c
last night but dropped it halfway, after finding one puts()
misconversion and fixing it. The changes should mostly be
straightforward but the result felt ugly.
next prev parent reply other threads:[~2007-11-22 19:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-22 2:54 [PATCH v2] builtin-commit: Include the diff in the commit message when verbose Kristian Høgsberg
2007-11-22 10:52 ` Johannes Schindelin
2007-11-22 11:13 ` Jeff King
2007-11-22 19:14 ` Junio C Hamano [this message]
2007-11-26 15:21 ` Kristian Høgsberg
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=7vzlx63xey.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=krh@redhat.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).