[PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s
In an intermediate iteration of my rebase--helper patches, I
accidentally generated commits with more than one empty line
between the header and the commit message. When using
find_commit_subject() to show the oneline, it turned up empty, even
if the output of `git show --format=%s` looked fine.
Turned out that the pretty-printing machinery helpfully skipped any
blank lines before the commit message.
In the first iteration of this patch, I failed to notice that
the skip_empty_lines() function used by the pretty printing (which is
declared static, and therefore I originally did not use it in order to
keep the patch as minimal as possible) skips also blank lines.
To make things truly consistent, I now just make the skip_empty_lines()
function public, and then use it.
Johannes Schindelin (2):
Make the skip_empty_lines() function public
Make find_commit_subject() more robust
commit.c | 2 +-
commit.h | 1 +
pretty.c | 2 +-
t/t8008-blame-formats.sh | 17 +
4 files changed, 20 insertions(+), 2 deletions(-)
Published-As: https://github.com/dscho/git/releases/tag/leading-empty-lines-v3
Interdiff vs v2:
diff --git a/commit.c b/commit.c
index 7b00989..0bf868f 100644
--- a/commit.c
+++ b/commit.c
@@ -414,9 +414,7 @@ int find_commit_subject(const char *commit_buffer, const
char **subject)
while (*p && (*p != '\n' || p[1] != '\n'))
p++;
if (*p) {
- p += 2;
- while (*p == '\n')
- p++;
+ p = skip_empty_lines(p + 2);
for (eol = p; *eol && *eol != '\n'; eol++)
; /* do nothing */
} else
diff --git a/commit.h b/commit.h
index b06db4d..fbdd18d 100644
--- a/commit.h
+++ b/commit.h
@@ -177,6 +177,7 @@ extern const char *format_subject(struct strbuf *sb, const
char *msg,
const char *line_separator);
extern void userformat_find_requirements(const char *fmt, struct
userformat_want *w);
extern int commit_format_is_empty(enum cmit_fmt);
+extern const char *skip_empty_lines(const char *msg);
extern void format_commit_message(const struct commit *commit,
const char *format, struct strbuf *sb,
const struct pretty_print_context *context);
diff --git a/pretty.c b/pretty.c
index c3ec430..1b807b4 100644
--- a/pretty.c
+++ b/pretty.c
@@ -516,7 +516,7 @@ static int is_empty_line(const char *line, int *len_p)
return !len;
}
-static const char *skip_empty_lines(const char *msg)
+const char *skip_empty_lines(const char *msg)
{
for (;;) {
int linelen = get_one_line(msg);
diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh
index 03bd313..b98f9a4 100755
--- a/t/t8008-blame-formats.sh
+++ b/t/t8008-blame-formats.sh
@@ -94,7 +94,7 @@ test_expect_success '--porcelain detects first non-empty
line as subject' '
echo "This is it" >single-file &&
git add single-file &&
tree=$(git write-tree) &&
- commit=$(printf "%s\n%s\n%s\n\n\noneline\n\nbody\n" \
+ commit=$(printf "%s\n%s\n%s\n\n\n \noneline\n\nbody\n" \
"tree $tree" \
"author A 123456789 +" \
"committer C 123456789 +" |
--
2.9.0.118.g0e1a633
base-commit: ab7797dbe95fff38d9265869ea367020046db118
--
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: [PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s
Johannes Schindelin writes: > In an intermediate iteration of my rebase--helper patches, I > accidentally generated commits with more than one empty line > between the header and the commit message. When using > find_commit_subject() to show the oneline, it turned up empty, even > if the output of `git show --format=%s` looked fine. Much easier to read with s/this developer/I/g ;-) > Turned out that the pretty-printing machinery helpfully skipped any > blank lines before the commit message. > > In the first iteration of this patch, I failed to notice that > the skip_empty_lines() function used by the pretty printing (which is > declared static, and therefore I originally did not use it in order to > keep the patch as minimal as possible) skips also blank lines. By the way, I think skip_empty_lines() is misnamed, and I think your use of "blank lines" in the previous paragraph indicates that you agree ;-) It probably was OK back when it was a file-local static helper in pretty.c, but it becomes a part of the global API with 1/2, renaming it to skip_blank_lines() may be a good thing to do there at the same time. I could do the tweaking while queuing if you too think it should happen; that way we'd save one roundtrip ;-). > To make things truly consistent, I now just make the skip_empty_lines() > function public, and then use it. Thanks. Interdiff looks sensible. -- 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: [PATCH v3 0/2] Make find_commit_subject() consistent with --format=%s
Hi Junio, On Wed, 22 Jun 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > In an intermediate iteration of my rebase--helper patches, I > > accidentally generated commits with more than one empty line > > between the header and the commit message. When using > > find_commit_subject() to show the oneline, it turned up empty, even > > if the output of `git show --format=%s` looked fine. > > Much easier to read with s/this developer/I/g ;-) :-P > > Turned out that the pretty-printing machinery helpfully skipped any > > blank lines before the commit message. > > > > In the first iteration of this patch, I failed to notice that > > the skip_empty_lines() function used by the pretty printing (which is > > declared static, and therefore I originally did not use it in order to > > keep the patch as minimal as possible) skips also blank lines. > > By the way, I think skip_empty_lines() is misnamed, and I think your > use of "blank lines" in the previous paragraph indicates that you > agree ;-) It probably was OK back when it was a file-local static > helper in pretty.c, but it becomes a part of the global API with > 1/2, renaming it to skip_blank_lines() may be a good thing to do > there at the same time. > > I could do the tweaking while queuing if you too think it should > happen; that way we'd save one roundtrip ;-). I just sent another iteration. Ciao, Dscho -- 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
