[PATCH/RFC] rev-list: fix --pretty=oneline with empty message

[PATCH/RFC] rev-list: fix --pretty=oneline with empty message

[Erik Faye-Lund]

55246aa (Dont use "<unknown>" for placeholders and suppress printing
of empty user formats) introduced a check to prevent empty
user-formats from being printed. This test didn't take empty commit
messages into account, and prevented the line-termination from being
output. This lead to multiple commits on a single line.

Correct it by guarding with a check for user-format.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---

This is a fix for the issue I posted about with rebase -i:
http://permalink.gmane.org/gmane.comp.version-control.git/141782

 builtin/rev-list.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 5679170..133056b 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -135,7 +135,8 @@ static void show_commit(struct commit *commit, void *data)
 					putchar('\n');
 			}
 		} else {
-			if (buf.len)
+			if (revs->commit_format != CMIT_FMT_USERFORMAT ||
+			    buf.len)
 				printf("%s%c", buf.buf, info->hdr_termination);
 		}
 		strbuf_release(&buf);
-- 
1.7.0.2.456.g64f24

Re: [PATCH/RFC] rev-list: fix --pretty=oneline with empty message

[Junio C Hamano]

Erik Faye-Lund <kusmabite@googlemail.com> writes:

> This is a fix for the issue I posted about with rebase -i:
> http://permalink.gmane.org/gmane.comp.version-control.git/141782

Thanks.

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 5679170..133056b 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -135,7 +135,8 @@ static void show_commit(struct commit *commit, void *data)
>  					putchar('\n');
>  			}
>  		} else {
> -			if (buf.len)
> +			if (revs->commit_format != CMIT_FMT_USERFORMAT ||
> +			    buf.len)
>  				printf("%s%c", buf.buf, info->hdr_termination);
>  		}

When the format is not user-specified one (e.g. "-v"), then a commit with
no message will only show the regular header, without any body.

I wonder if we need something similar in the "graph" case, most of which
is outside the context before this hunk.  With your patch, this still
seems to misbehave:

    $ git rev-list --graph --oneline --abbrev=4 HEAD
    * 8cb0 * efea 4th commit
    * 341e 3rd commit
    * 2321 2nd commit
    * 3814 1st commit

Although anything other than --oneline seems to do just fine.

Perhaps this one on top, with an appropriate test, and we can lose the RFC
from the title?

 builtin-rev-list.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 133056b..1db4b4c 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -133,6 +133,8 @@ static void show_commit(struct commit *commit, void *data)
 				 */
 				if (graph_show_remainder(revs->graph))
 					putchar('\n');
+				if (revs->commit_format == CMIT_FMT_ONELINE)
+					putchar('\n');
 			}
 		} else {
 			if (revs->commit_format != CMIT_FMT_USERFORMAT ||