git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] log_tree_diff: simplify
@ 2020-08-28 11:05 Sergey Organov
  2020-08-28 11:05 ` [PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only Sergey Organov
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Sergey Organov @ 2020-08-28 11:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Sergey Organov

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(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only
  2020-08-28 11:05 [PATCH 0/2] log_tree_diff: simplify Sergey Organov
@ 2020-08-28 11:05 ` Sergey Organov
  2020-08-28 11:05 ` [PATCH 2/2] log_tree_diff: get rid of extra check for NULL Sergey Organov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Sergey Organov @ 2020-08-28 11:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Sergey Organov

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] log_tree_diff: get rid of extra check for NULL
  2020-08-28 11:05 [PATCH 0/2] log_tree_diff: simplify Sergey Organov
  2020-08-28 11:05 ` [PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only Sergey Organov
@ 2020-08-28 11:05 ` Sergey Organov
  2020-09-04 12:38 ` [PATCH 0/2] log_tree_diff: simplify Sergey Organov
  2020-09-06 21:52 ` Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Sergey Organov @ 2020-08-28 11:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Sergey Organov

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] log_tree_diff: simplify
  2020-08-28 11:05 [PATCH 0/2] log_tree_diff: simplify Sergey Organov
  2020-08-28 11:05 ` [PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only Sergey Organov
  2020-08-28 11:05 ` [PATCH 2/2] log_tree_diff: get rid of extra check for NULL Sergey Organov
@ 2020-09-04 12:38 ` Sergey Organov
  2020-09-06 21:52 ` Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Sergey Organov @ 2020-09-04 12:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] log_tree_diff: simplify
  2020-08-28 11:05 [PATCH 0/2] log_tree_diff: simplify Sergey Organov
                   ` (2 preceding siblings ...)
  2020-09-04 12:38 ` [PATCH 0/2] log_tree_diff: simplify Sergey Organov
@ 2020-09-06 21:52 ` Junio C Hamano
  3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2020-09-06 21:52 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Jeff King, git

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-06 21:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 11:05 [PATCH 0/2] log_tree_diff: simplify Sergey Organov
2020-08-28 11:05 ` [PATCH 1/2] log_tree_diff: get rid of code duplication for first_parent_only Sergey Organov
2020-08-28 11:05 ` [PATCH 2/2] log_tree_diff: get rid of extra check for NULL Sergey Organov
2020-09-04 12:38 ` [PATCH 0/2] log_tree_diff: simplify Sergey Organov
2020-09-06 21:52 ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git