Bug? diff.submodule=log adds text to commit -v message
Hi, I'm using git 1.8.4.2, and I've set the "diff.submodule = log" option globally. If I change the revision that a submodule is set to, then run "git commit -av", The submodule shortlog is appended to the log message without any #s before it, so the log messages get included in my own log message. This seems like a bug and not a feature, as diffs aren't normally included in the commit message with -v. Cheers, Ari -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Hi Ari, Am 10.11.2013 22:49, schrieb Ari Pollak: > I'm using git 1.8.4.2, and I've set the "diff.submodule = log" option > globally. If I change the revision that a submodule is set to, then run > "git commit -av", The submodule shortlog is appended to the log message > without > any #s before it, so the log messages get included in my own log message. > This seems like a bug and not a feature, as diffs aren't normally included in > the commit message with -v. Thanks for your report, I can reproduce that here. But first I think this is unrelated to the "diff.submodule = log" setting, as without it you'll just see the submodule commit hash diff instead of the shortlog (which is perfectly consistent with what I'd expect from this setting). And secondly what you describe looks like documented behavior, the man page of "git commit" states: -v, --verbose Show unified diff between the HEAD commit and what would be committed at the bottom of the commit message template. Note that this diff output doesn't have its lines prefixed with #. And after adding a modified file the log message also shows the diff of that file (and without leading "# "s too), so I doubt that diffs aren't normally included in the commit message with -v. What am I missing? Thanks Jens -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Jens Lehmann writes: > And after adding a modified file the log message also shows the diff of > that file (and without leading "# "s too), so I doubt that diffs aren't > normally included in the commit message with -v. What am I missing? Ah, it is true that -v normally does not prefix the diffs with #, but there must be some filter in place after I save & quit my editor when a normal file diff is present, since that does not get included in the final commit message. But the submodule log does get included, which does not seem intentional. Cheers, Ari -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Am 11.11.2013 21:48, schrieb Ari Pollak:
> Jens Lehmann writes:
>> And after adding a modified file the log message also shows the diff of
>> that file (and without leading "# "s too), so I doubt that diffs aren't
>> normally included in the commit message with -v. What am I missing?
>
> Ah, it is true that -v normally does not prefix the diffs with #, but there
> must be some filter in place after I save & quit my editor when a normal file
> diff is present, since that does not get included in the final commit
> message. But the submodule log does get included, which does not seem
> intentional.
Ok, now this makes sense. "git commit" strips off the diff added by
-v by skipping everything starting with "\ndiff --git ". But that
logic fails when the "diff.submodule = log" setting adds a shortlog
instead of a regular diff, as that starts with "\nSubmodule ".
The diff below fixes the problem you describe for me. (But I do not
consider it a worthwhile fix in its current form because a line
starting with "Submodule " might appear in a perfectly normal commit
message, while "diff --git " most probably won't).
And while testing this issue I noticed another problem: When using
"git commit -a" not only the staged commits of a submodule get
committed, but also the unstaged commits. Will look into that too.
-8<-
diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..ff6e171 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1600,11 +1600,14 @@ int cmd_commit(int argc, const char **argv, const char *
die(_("could not read commit message: %s"), strerror(saved_errno
}
- /* Truncate the message just before the diff, if any. */
+ /* Truncate the message just before the diff or submodule shortlog */
if (verbose) {
p = strstr(sb.buf, "\ndiff --git ");
if (p != NULL)
strbuf_setlen(&sb, p - sb.buf + 1);
+ p = strstr(sb.buf, "\nSubmodule ");
+ if (p != NULL)
+ strbuf_setlen(&sb, p - sb.buf + 1);
}
if (cleanup_mode != CLEANUP_NONE)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Am 11.11.2013 22:29, schrieb Jens Lehmann: > And while testing this issue I noticed another problem: When using > "git commit -a" not only the staged commits of a submodule get > committed, but also the unstaged commits. Will look into that too. Ok, scrap that. This is exactly what is expected. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
On Mon, Nov 11, 2013 at 10:29:25PM +0100, Jens Lehmann wrote: > The diff below fixes the problem you describe for me. (But I do not > consider it a worthwhile fix in its current form because a line > starting with "Submodule " might appear in a perfectly normal commit > message, while "diff --git " most probably won't). Yeah, this fix makes me nervous for that reason. "commit -v" has always been a little bit flaky in that respect, as it is simply guessing at the beginning of the diff text it added earlier. In addition to false negatives, it also has false positives, stripping out people's diffs that they meant to include in the commit message. The "right" way to fix this is to change the format to use some more robust marker, like: # Everything below this line is a diff that will be removed. I do not know offhand if anybody's commit-template generating or parsing scripts would be broken, but I doubt the fallout would be that big. When last we discussed this (AFAICT), we did not yet have 0b38227 (commit: Fix stripping of patch in verbose mode., 2008-11-12), which meant that it would affect _everybody_. Nowadays it would only affect users of "-v", which is presumably a much smaller population. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Am 11.11.2013 22:29, schrieb Jens Lehmann: > The diff below fixes the problem you describe for me. (But I do not > consider it a worthwhile fix in its current form because a line > starting with "Submodule " might appear in a perfectly normal commit > message, while "diff --git " most probably won't). And on top of that, "Submodule " originates from a translatable string, doesn't it? -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Am 12.11.2013 08:46, schrieb Johannes Sixt: > Am 11.11.2013 22:29, schrieb Jens Lehmann: >> The diff below fixes the problem you describe for me. (But I do not >> consider it a worthwhile fix in its current form because a line >> starting with "Submodule " might appear in a perfectly normal commit >> message, while "diff --git " most probably won't). > > And on top of that, "Submodule " originates from a translatable string, > doesn't it? This would also be true for the marker line that Peff proposed: # Everything below this line is a diff that will be removed. But I suspect that would be ok if the marker would be both added and searched for in its translated form. Or is it possible that the locale changes between those two steps? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Jens Lehmann writes: > Am 12.11.2013 08:46, schrieb Johannes Sixt: >> Am 11.11.2013 22:29, schrieb Jens Lehmann: >>> The diff below fixes the problem you describe for me. (But I do not >>> consider it a worthwhile fix in its current form because a line >>> starting with "Submodule " might appear in a perfectly normal commit >>> message, while "diff --git " most probably won't). >> >> And on top of that, "Submodule " originates from a translatable string, >> doesn't it? > > This would also be true for the marker line that Peff proposed: > > # Everything below this line is a diff that will be removed. > > But I suspect that would be ok if the marker would be both added > and searched for in its translated form. Or is it possible that > the locale changes between those two steps? If we were introducing a divider line for machine consumption, I do not think it is wise to let that line even translated... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Am 12.11.2013 23:20, schrieb Junio C Hamano: > Jens Lehmann writes: > >> Am 12.11.2013 08:46, schrieb Johannes Sixt: >>> Am 11.11.2013 22:29, schrieb Jens Lehmann: The diff below fixes the problem you describe for me. (But I do not consider it a worthwhile fix in its current form because a line starting with "Submodule " might appear in a perfectly normal commit message, while "diff --git " most probably won't). >>> >>> And on top of that, "Submodule " originates from a translatable string, >>> doesn't it? >> >> This would also be true for the marker line that Peff proposed: >> >> # Everything below this line is a diff that will be removed. >> >> But I suspect that would be ok if the marker would be both added >> and searched for in its translated form. Or is it possible that >> the locale changes between those two steps? > > If we were introducing a divider line for machine consumption, I do > not think it is wise to let that line even translated... Ok, but then it won't mean much to readers who don't understand English. I assume prefixing all diff lines with "# " is out of the question because of backwards compatibility, so what about using a descriptive text together with a scissor line? The former can be be translated (and won't make it into the commit message because it starts with a "#") while the latter serves as a robust divider line: # Everything below the following line is a diff that will be removed. # 8< -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug? diff.submodule=log adds text to commit -v message
Jens Lehmann writes: >> If we were introducing a divider line for machine consumption, I do >> not think it is wise to let that line even translated... > > Ok, but then it won't mean much to readers who don't understand > English. I assume prefixing all diff lines with "# " is out of > the question because of backwards compatibility, so what about > using a descriptive text together with a scissor line? The former > can be be translated (and won't make it into the commit message > because it starts with a "#") while the latter serves as a robust > divider line: > > # Everything below the following line is a diff that will be removed. > # 8< Yeah, or swap them around if you are trying to protect the part above the divider from getting contaminated by the noise. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
