Re: [PATCH v3 3/3] commit::print_summary(): set rev_info.always_show_header to 1
Hi Will,
sorry, I forgot to Cc you for this new version.
--
Cheers,
Ray Chuan
On Fri, Jun 4, 2010 at 4:21 PM, Tay Ray Chuan <rctay89@gmail.com> wrote:
> This attempts to fix a regression in git-commit, where non-abbreviated
> SHA-1s were printed in the summary.
>
> One possible fix would be to set ctx.abbrev to DEFAULT_ABBREV in the
> `if` block.
>
> However, we remove this codeblock altogether, and set
> rev.always_show_header. This way, we use back the same show_log()
> mechanism (instead of format_commit_message()).
>
> Quoting log-tree.c:560:
>
> shown = log_tree_diff(opt, commit, &log);
> if (!shown && opt->loginfo && opt->always_show_header) {
> log.parent = NULL;
> show_log(opt);
> shown = 1;
> }
>
> This is the only area that always_show_header is checked, so the
> setting of this flag should only affect this area.
>
> Also, we now die() if log_tree_commit() returns false. Based on the
> existing underlying codepaths (log_tree_commit(), log_tree_diff(),
> log_tree_diff_flush(), to name a few), this should not occur; changes to
> these codepaths may warrant a revision of our handling of this
> situation. Tests #2 and #3 in t7502 should aid in detecting such
> breakages.
>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>
> This is a reworked version of the third patch of the
> 'tc/commit-abbrev-fix' series; there are no changes to the first and
> second patches.
>
> Changes from v2:
> - shift around 2nd and 3rd paras.
> - mention the die() and our assumption that it won't be triggered, as
> suggested by Junio.
>
> builtin/commit.c | 13 ++++---------
> t/t7502-commit.sh | 4 ++--
> 2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index a4e4966..2884d0c 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1148,7 +1148,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
> rev.verbose_header = 1;
> rev.show_root_diff = 1;
> get_commit_format(format.buf, &rev);
> - rev.always_show_header = 0;
> + rev.always_show_header = 1;
> rev.diffopt.detect_rename = 1;
> rev.diffopt.rename_limit = 100;
> rev.diffopt.break_opt = 0;
> @@ -1162,14 +1162,9 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
> head,
> initial_commit ? " (root-commit)" : "");
>
> - if (!log_tree_commit(&rev, commit)) {
> - struct pretty_print_context ctx = {0};
> - struct strbuf buf = STRBUF_INIT;
> - ctx.date_mode = DATE_NORMAL;
> - format_commit_message(commit, format.buf + 7, &buf, &ctx);
> - printf("%s\n", buf.buf);
> - strbuf_release(&buf);
> - }
> + if (!log_tree_commit(&rev, commit))
> + die("unable to print summary");
> +
> strbuf_release(&format);
> }
>
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index b10541d..08c0247 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -36,12 +36,12 @@ test_expect_success 'output summary format' '
> check_summary_oneline "" "a change"
> '
>
> -test_expect_failure 'output summary format for commit with an empty diff' '
> +test_expect_success 'output summary format for commit with an empty diff' '
>
> check_summary_oneline "" "empty" "--allow-empty"
> '
>
> -test_expect_failure 'output summary format for merges' '
> +test_expect_success 'output summary format for merges' '
>
> git checkout -b recursive-base &&
> test_commit base file1 &&
> --
> 1.7.1.189.g07419
>
>Tay Ray Chuan <rctay89@gmail.com>