git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH] commit -v: strip diffs and submodule shortlogs from the commit message

From
Jens Lehmann <jens.lehmann@web.de>
Date
Nov 17, 2013, 12:20 UTC
Message-ID
<5288B49D.8080401@web.de>
In-Reply-To
<20131117090935.GC17016@sigill.intra.peff.net>
Am 17.11.2013 10:09, schrieb Jeff King:
Show 24 quoted lines
> 
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 6ab4605..091a6e7 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1602,9 +1602,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>
>>  	/* Truncate the message just before the diff, if any. */
>>  	if (verbose) {
>> -		p = strstr(sb.buf, "\ndiff --git ");
>> -		if (p != NULL)
>> -			strbuf_setlen(&sb, p - sb.buf + 1);
>> +		p = strstr(sb.buf, wt_status_diff_divider);
>> +		if ((p != NULL) && (p > sb.buf) && (p[-1] == '\n'))
>> +			strbuf_setlen(&sb, p - sb.buf);
> 
> I think your check for a preceding newline is too strict. If I delete
> everything before the scissor line (e.g., because I am trying to abort
> the commit), we should still remove the diff. With your patch, we do
> not, and a commit message consisting solely of the diff.
> 
> So I think you want:
> 
>   if (p && (p == sb.buf || p[-1] == '\n'))
Thanks for catching this, will do so in v2.
Show 11 quoted lines
>> +	fprintf(s->fp, _("# The diff below will be removed when keeping the previous line.\n"));
> 
> I found this hard to parse, I think because of the "keeping" (why would
> I not keep it?), and because you are talking about lines above and
> below. It is not as accurate to say:
> 
>   # ------------------ >8 --------------------
>   # Everything below this line will be removed.
> 
> because it is technically the line above that is the cutoff. But I think
> the meaning is clear, and it is simpler to parse.
Ok.
Show 17 quoted lines
> I do think it would be simpler with a single line. I know handling the
> i18n was a question there, but I think we should be fine as long as we
> check for the exact bytes we wrote. Surely gettext can do something
> like:
> 
>   magic = _("# Everything below this line will be removed");
>   fprintf(fh, "%s", magic);
>   ...
>   p = strstr(magic);
> 
> I don't know what guarantees on string lifetime gettext gives us, but
> the worst case is that we simply strdup the result.
> 
> I suppose it's possible that the translated string could have utf8 with
> multiple representations, and the user's editor normalizes the text in a
> different way than we wrote it when it saves the result. I don't know if
> that is worth caring about or not; it seems kind of insane.

I don't have any strong feelings about this one. I'd be fine with dropping the scissor line and taking the translated string as divider line. What do others think?

Previous: Jeff KingNext: Junio C Hamano
Message 16 of 23 in “Bug? diff.submodule=log adds text to commit -v message”
  1. Ari PollakNov 10, 2013
  2. Jens LehmannNov 11, 2013
  3. Ari PollakNov 11, 2013
  4. Jens LehmannNov 11, 2013
  5. Jens LehmannNov 11, 2013
  6. Jeff KingNov 11, 2013
  7. Johannes SixtNov 12, 2013
  8. Jens LehmannNov 12, 2013
  9. Junio C HamanoNov 12, 2013
  10. Jens LehmannNov 13, 2013
  11. Junio C HamanoNov 13, 2013
  12. commit -v: strip diffs and submodule shortlogs from the commit messageJens Lehmann, Nov 16, 2013
  13. Eric SunshineNov 17, 2013
  14. Jeff KingNov 17, 2013
  15. Jeff KingNov 17, 2013
  16. Jens LehmannNov 17, 2013
  17. Junio C HamanoNov 18, 2013
  18. commit -v: strip diffs and submodule shortlogs from the commit messageJens Lehmann, Nov 19, 2013
  19. Junio C HamanoNov 19, 2013
  20. Junio C HamanoNov 19, 2013
  21. Junio C HamanoNov 19, 2013
  22. Junio C HamanoNov 19, 2013
  23. Jens LehmannNov 19, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.