threads / patch / 54149
Subject: [PATCH 0/2] log_tree_diff: simplify
Sergey OrganovThese patches are pure code quality improvements, -- they should introduce no changes in behavior.
Sergey Organov (2): log_tree_diff: get rid of code duplication for first_parent_only log_tree_diff: get rid of extra check for NULL
log-tree.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)
-- 2.25.1
Sergey Organov[PATCH 2/2] log_tree_diff: get rid of extra check for NULL
Get rid of needless check of 'parents' for NULL. The NULL case is already handled right above, and 'parents' is dereferenced without check below anyway.
Signed-off-by: Sergey Organov <sorganov@gmail.com> --- log-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/log-tree.c b/log-tree.c index c01932fa72bf..8ac285a25af5 100644 --- a/log-tree.c +++ b/log-tree.c @@ -917,7 +917,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log } /* More than one parent? */ - if (parents && parents->next) { + if (parents->next) { if (opt->ignore_merges) return 0; else if (opt->combine_merges)
-- 2.25.1
Sergey Organov[PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only
Handle first_parent_only by breaking from generic loop early rather than by duplicating (part of) the loop body.
Signed-off-by: Sergey Organov <sorganov@gmail.com> --- log-tree.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/log-tree.c b/log-tree.c index 55a68d0c6101..c01932fa72bf 100644 --- a/log-tree.c +++ b/log-tree.c @@ -922,21 +922,10 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log return 0; else if (opt->combine_merges) return do_diff_combined(opt, commit); - else if (opt->first_parent_only) { - /* - * Generate merge log entry only for the first - * parent, showing summary diff of the others - * we merged _in_. - */ - parse_commit_or_die(parents->item); - diff_tree_oid(get_commit_tree_oid(parents->item), - oid, "", &opt->diffopt); - log_tree_diff_flush(opt); - return !opt->loginfo; + else if (!opt->first_parent_only) { + /* If we show multiple diffs, show the parent info */ + log->parent = parents->item; } - - /* If we show individual diffs, show the parent info */ - log->parent = parents->item; } showed_log = 0; @@ -952,7 +941,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log /* Set up the log info for the next parent, if any.. */ parents = parents->next; - if (!parents) + if (!parents || opt->first_parent_only) break; log->parent = parents->item; opt->loginfo = log;
-- 2.25.1
Sergey OrganovRe: [PATCH 0/2] log_tree_diff: simplify
Dear Junio,
Sergey Organov <sorganov@gmail.com> writes:
> These patches are pure code quality improvements, -- they should introduce no > changes in behavior. > > Sergey Organov (2): > log_tree_diff: get rid of code duplication for first_parent_only > log_tree_diff: get rid of extra check for NULL
What's the status of this?
Thanks, -- Sergey Organov
Junio C HamanoRe: [PATCH 0/2] log_tree_diff: simplify
Sergey Organov <sorganov@gmail.com> writes:
> These patches are pure code quality improvements, -- they should introduce no > changes in behavior. > > Sergey Organov (2): > log_tree_diff: get rid of code duplication for first_parent_only > log_tree_diff: get rid of extra check for NULL > > log-tree.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-)
It's been so long since I looked at the codepath that it wasn't immediately clear to me if we are doing the right thing around log->parent until I reread 91539833 (Log message printout cleanups, 2006-04-17) to rediscover what the field was for, but everything looks good to me.