git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] diff: get rid of redundant 'dense' argument
@ 2020-09-29 11:31 Sergey Organov
  2020-09-29 18:54 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Sergey Organov @ 2020-09-29 11:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Sergey Organov

Get rid of 'dense' argument that is redundant for every function that has
'struct rev_info *rev' argument as well, as the value of 'dense' passed is
always taken from 'rev->dense_combined_merges' field.

The only place where this was not the case is in 'submodule.c' where
'diff_tree_combined_merge()' was called with '1' for 'dense' argument. However,
at that call the 'revs' instance used is local to the function, and we now just
set 'revs->dense_combined_merges' to 1 in this local instance.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 builtin/diff.c |  3 +--
 combine-diff.c | 21 +++++++++------------
 diff-lib.c     |  6 ++----
 diff.h         |  6 +++---
 log-tree.c     |  2 +-
 submodule.c    |  3 ++-
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index cb98811c21db..cd4083fed96e 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -203,8 +203,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	for (i = 1; i < ents; i++)
 		oid_array_append(&parents, &ent[i].item->oid);
-	diff_tree_combined(&ent[0].item->oid, &parents,
-			   revs->dense_combined_merges, revs);
+	diff_tree_combined(&ent[0].item->oid, &parents, revs);
 	oid_array_clear(&parents);
 	return 0;
 }
diff --git a/combine-diff.c b/combine-diff.c
index 002e0e5438bc..555b812a9975 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -923,7 +923,6 @@ static void dump_quoted_path(const char *head,
 
 static void show_combined_header(struct combine_diff_path *elem,
 				 int num_parent,
-				 int dense,
 				 struct rev_info *rev,
 				 const char *line_prefix,
 				 int mode_differs,
@@ -939,6 +938,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 	int added = 0;
 	int deleted = 0;
 	int i;
+	int dense = rev->dense_combined_merges;
 
 	if (rev->loginfo && !rev->no_commit_id)
 		show_log(rev);
@@ -1012,7 +1012,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 }
 
 static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
-			    int dense, int working_tree_file,
+			    int working_tree_file,
 			    struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -1145,7 +1145,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		}
 	}
 	if (is_binary) {
-		show_combined_header(elem, num_parent, dense, rev,
+		show_combined_header(elem, num_parent, rev,
 				     line_prefix, mode_differs, 0);
 		printf("Binary files differ\n");
 		free(result);
@@ -1200,10 +1200,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 				     textconv, elem->path, opt->xdl_opts);
 	}
 
-	show_hunks = make_hunks(sline, cnt, num_parent, dense);
+	show_hunks = make_hunks(sline, cnt, num_parent, rev->dense_combined_merges);
 
 	if (show_hunks || mode_differs || working_tree_file) {
-		show_combined_header(elem, num_parent, dense, rev,
+		show_combined_header(elem, num_parent, rev,
 				     line_prefix, mode_differs, 1);
 		dump_sline(sline, line_prefix, cnt, num_parent,
 			   opt->use_color, result_deleted);
@@ -1284,7 +1284,6 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
  */
 void show_combined_diff(struct combine_diff_path *p,
 		       int num_parent,
-		       int dense,
 		       struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -1294,7 +1293,7 @@ void show_combined_diff(struct combine_diff_path *p,
 				  DIFF_FORMAT_NAME_STATUS))
 		show_raw_diff(p, num_parent, rev);
 	else if (opt->output_format & DIFF_FORMAT_PATCH)
-		show_patch_diff(p, num_parent, dense, 1, rev);
+		show_patch_diff(p, num_parent, 1, rev);
 }
 
 static void free_combined_pair(struct diff_filepair *pair)
@@ -1454,7 +1453,6 @@ static struct combine_diff_path *find_paths_multitree(
 
 void diff_tree_combined(const struct object_id *oid,
 			const struct oid_array *parents,
-			int dense,
 			struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -1581,8 +1579,7 @@ void diff_tree_combined(const struct object_id *oid,
 				printf("%s%c", diff_line_prefix(opt),
 				       opt->line_termination);
 			for (p = paths; p; p = p->next)
-				show_patch_diff(p, num_parent, dense,
-						0, rev);
+				show_patch_diff(p, num_parent, 0, rev);
 		}
 	}
 
@@ -1600,7 +1597,7 @@ void diff_tree_combined(const struct object_id *oid,
 	clear_pathspec(&diffopts.pathspec);
 }
 
-void diff_tree_combined_merge(const struct commit *commit, int dense,
+void diff_tree_combined_merge(const struct commit *commit,
 			      struct rev_info *rev)
 {
 	struct commit_list *parent = get_saved_parents(rev, commit);
@@ -1610,6 +1607,6 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
 		oid_array_append(&parents, &parent->item->object.oid);
 		parent = parent->next;
 	}
-	diff_tree_combined(&commit->object.oid, &parents, dense, rev);
+	diff_tree_combined(&commit->object.oid, &parents, rev);
 	oid_array_clear(&parents);
 }
diff --git a/diff-lib.c b/diff-lib.c
index 346fdcf0b0ce..f95c6de75fc8 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -177,9 +177,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			i--;
 
 			if (revs->combine_merges && num_compare_stages == 2) {
-				show_combined_diff(dpath, 2,
-						   revs->dense_combined_merges,
-						   revs);
+				show_combined_diff(dpath, 2, revs);
 				free(dpath);
 				continue;
 			}
@@ -361,7 +359,7 @@ static int show_modified(struct rev_info *revs,
 		p->parent[1].status = DIFF_STATUS_MODIFIED;
 		p->parent[1].mode = old_entry->ce_mode;
 		oidcpy(&p->parent[1].oid, &old_entry->oid);
-		show_combined_diff(p, 2, revs->dense_combined_merges, revs);
+		show_combined_diff(p, 2, revs);
 		free(p);
 		return 0;
 	}
diff --git a/diff.h b/diff.h
index 49242d2733c0..dc6e09a55e62 100644
--- a/diff.h
+++ b/diff.h
@@ -454,11 +454,11 @@ struct combine_diff_path {
 		st_mult(sizeof(struct combine_diff_parent), (n)))
 
 void show_combined_diff(struct combine_diff_path *elem, int num_parent,
-			int dense, struct rev_info *);
+			struct rev_info *);
 
-void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, int dense, struct rev_info *rev);
+void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, struct rev_info *rev);
 
-void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
+void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);
 
 void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
 
diff --git a/log-tree.c b/log-tree.c
index cb8942fec181..1927f917ce94 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -885,7 +885,7 @@ int log_tree_diff_flush(struct rev_info *opt)
 
 static int do_diff_combined(struct rev_info *opt, struct commit *commit)
 {
-	diff_tree_combined_merge(commit, opt->dense_combined_merges, opt);
+	diff_tree_combined_merge(commit, opt);
 	return !opt->loginfo;
 }
 
diff --git a/submodule.c b/submodule.c
index 543b1123ae12..b3bb59f06644 100644
--- a/submodule.c
+++ b/submodule.c
@@ -865,7 +865,8 @@ static void collect_changed_submodules(struct repository *r,
 		diff_rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 		diff_rev.diffopt.format_callback = collect_changed_submodules_cb;
 		diff_rev.diffopt.format_callback_data = &data;
-		diff_tree_combined_merge(commit, 1, &diff_rev);
+		diff_rev.dense_combined_merges = 1;
+		diff_tree_combined_merge(commit, &diff_rev);
 	}
 
 	reset_revision_walk();
-- 
2.25.1


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

* Re: [PATCH] diff: get rid of redundant 'dense' argument
  2020-09-29 11:31 [PATCH] diff: get rid of redundant 'dense' argument Sergey Organov
@ 2020-09-29 18:54 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2020-09-29 18:54 UTC (permalink / raw)
  To: Sergey Organov; +Cc: Jeff King, git

Sergey Organov <sorganov@gmail.com> writes:

> Get rid of 'dense' argument that is redundant for every function that has
> 'struct rev_info *rev' argument as well, as the value of 'dense' passed is
> always taken from 'rev->dense_combined_merges' field.
>
> The only place where this was not the case is in 'submodule.c' where
> 'diff_tree_combined_merge()' was called with '1' for 'dense' argument. However,
> at that call the 'revs' instance used is local to the function, and we now just
> set 'revs->dense_combined_merges' to 1 in this local instance.

Interesting.  This dates back to 91539833 (Log message printout
cleanups, 2006-04-17), where show_patch_diff() in combine-diff.c
that used to take "struct diff_options *" was modified to take
"struct rev_info *".  I think the codepath took "int dense" from
the beginning of the combined diff feature and I suspect this
change could have been made in that old commit.

Looking at the history, it seems that it definititely could have
been noticed when I reorganized the codepath involved in combined
and dense combined patches in 0fe7c1de (built-in diff: assorted
updates., 2006-04-29).  Back when the callers of diff_tree_combined()
were only a few and it was more obvious that the singleton 'dense'
was (or would soon become) redundant.

Will queue.  Thanks.

> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  builtin/diff.c |  3 +--
>  combine-diff.c | 21 +++++++++------------
>  diff-lib.c     |  6 ++----
>  diff.h         |  6 +++---
>  log-tree.c     |  2 +-
>  submodule.c    |  3 ++-
>  6 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index cb98811c21db..cd4083fed96e 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -203,8 +203,7 @@ static int builtin_diff_combined(struct rev_info *revs,
>  		revs->dense_combined_merges = revs->combine_merges = 1;
>  	for (i = 1; i < ents; i++)
>  		oid_array_append(&parents, &ent[i].item->oid);
> -	diff_tree_combined(&ent[0].item->oid, &parents,
> -			   revs->dense_combined_merges, revs);
> +	diff_tree_combined(&ent[0].item->oid, &parents, revs);
>  	oid_array_clear(&parents);
>  	return 0;
>  }
> diff --git a/combine-diff.c b/combine-diff.c
> index 002e0e5438bc..555b812a9975 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -923,7 +923,6 @@ static void dump_quoted_path(const char *head,
>  
>  static void show_combined_header(struct combine_diff_path *elem,
>  				 int num_parent,
> -				 int dense,
>  				 struct rev_info *rev,
>  				 const char *line_prefix,
>  				 int mode_differs,
> @@ -939,6 +938,7 @@ static void show_combined_header(struct combine_diff_path *elem,
>  	int added = 0;
>  	int deleted = 0;
>  	int i;
> +	int dense = rev->dense_combined_merges;
>  
>  	if (rev->loginfo && !rev->no_commit_id)
>  		show_log(rev);
> @@ -1012,7 +1012,7 @@ static void show_combined_header(struct combine_diff_path *elem,
>  }
>  
>  static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
> -			    int dense, int working_tree_file,
> +			    int working_tree_file,
>  			    struct rev_info *rev)
>  {
>  	struct diff_options *opt = &rev->diffopt;
> @@ -1145,7 +1145,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
>  		}
>  	}
>  	if (is_binary) {
> -		show_combined_header(elem, num_parent, dense, rev,
> +		show_combined_header(elem, num_parent, rev,
>  				     line_prefix, mode_differs, 0);
>  		printf("Binary files differ\n");
>  		free(result);
> @@ -1200,10 +1200,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
>  				     textconv, elem->path, opt->xdl_opts);
>  	}
>  
> -	show_hunks = make_hunks(sline, cnt, num_parent, dense);
> +	show_hunks = make_hunks(sline, cnt, num_parent, rev->dense_combined_merges);
>  
>  	if (show_hunks || mode_differs || working_tree_file) {
> -		show_combined_header(elem, num_parent, dense, rev,
> +		show_combined_header(elem, num_parent, rev,
>  				     line_prefix, mode_differs, 1);
>  		dump_sline(sline, line_prefix, cnt, num_parent,
>  			   opt->use_color, result_deleted);
> @@ -1284,7 +1284,6 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
>   */
>  void show_combined_diff(struct combine_diff_path *p,
>  		       int num_parent,
> -		       int dense,
>  		       struct rev_info *rev)
>  {
>  	struct diff_options *opt = &rev->diffopt;
> @@ -1294,7 +1293,7 @@ void show_combined_diff(struct combine_diff_path *p,
>  				  DIFF_FORMAT_NAME_STATUS))
>  		show_raw_diff(p, num_parent, rev);
>  	else if (opt->output_format & DIFF_FORMAT_PATCH)
> -		show_patch_diff(p, num_parent, dense, 1, rev);
> +		show_patch_diff(p, num_parent, 1, rev);
>  }
>  
>  static void free_combined_pair(struct diff_filepair *pair)
> @@ -1454,7 +1453,6 @@ static struct combine_diff_path *find_paths_multitree(
>  
>  void diff_tree_combined(const struct object_id *oid,
>  			const struct oid_array *parents,
> -			int dense,
>  			struct rev_info *rev)
>  {
>  	struct diff_options *opt = &rev->diffopt;
> @@ -1581,8 +1579,7 @@ void diff_tree_combined(const struct object_id *oid,
>  				printf("%s%c", diff_line_prefix(opt),
>  				       opt->line_termination);
>  			for (p = paths; p; p = p->next)
> -				show_patch_diff(p, num_parent, dense,
> -						0, rev);
> +				show_patch_diff(p, num_parent, 0, rev);
>  		}
>  	}
>  
> @@ -1600,7 +1597,7 @@ void diff_tree_combined(const struct object_id *oid,
>  	clear_pathspec(&diffopts.pathspec);
>  }
>  
> -void diff_tree_combined_merge(const struct commit *commit, int dense,
> +void diff_tree_combined_merge(const struct commit *commit,
>  			      struct rev_info *rev)
>  {
>  	struct commit_list *parent = get_saved_parents(rev, commit);
> @@ -1610,6 +1607,6 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
>  		oid_array_append(&parents, &parent->item->object.oid);
>  		parent = parent->next;
>  	}
> -	diff_tree_combined(&commit->object.oid, &parents, dense, rev);
> +	diff_tree_combined(&commit->object.oid, &parents, rev);
>  	oid_array_clear(&parents);
>  }
> diff --git a/diff-lib.c b/diff-lib.c
> index 346fdcf0b0ce..f95c6de75fc8 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -177,9 +177,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>  			i--;
>  
>  			if (revs->combine_merges && num_compare_stages == 2) {
> -				show_combined_diff(dpath, 2,
> -						   revs->dense_combined_merges,
> -						   revs);
> +				show_combined_diff(dpath, 2, revs);
>  				free(dpath);
>  				continue;
>  			}
> @@ -361,7 +359,7 @@ static int show_modified(struct rev_info *revs,
>  		p->parent[1].status = DIFF_STATUS_MODIFIED;
>  		p->parent[1].mode = old_entry->ce_mode;
>  		oidcpy(&p->parent[1].oid, &old_entry->oid);
> -		show_combined_diff(p, 2, revs->dense_combined_merges, revs);
> +		show_combined_diff(p, 2, revs);
>  		free(p);
>  		return 0;
>  	}
> diff --git a/diff.h b/diff.h
> index 49242d2733c0..dc6e09a55e62 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -454,11 +454,11 @@ struct combine_diff_path {
>  		st_mult(sizeof(struct combine_diff_parent), (n)))
>  
>  void show_combined_diff(struct combine_diff_path *elem, int num_parent,
> -			int dense, struct rev_info *);
> +			struct rev_info *);
>  
> -void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, int dense, struct rev_info *rev);
> +void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, struct rev_info *rev);
>  
> -void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
> +void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);
>  
>  void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
>  
> diff --git a/log-tree.c b/log-tree.c
> index cb8942fec181..1927f917ce94 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -885,7 +885,7 @@ int log_tree_diff_flush(struct rev_info *opt)
>  
>  static int do_diff_combined(struct rev_info *opt, struct commit *commit)
>  {
> -	diff_tree_combined_merge(commit, opt->dense_combined_merges, opt);
> +	diff_tree_combined_merge(commit, opt);
>  	return !opt->loginfo;
>  }
>  
> diff --git a/submodule.c b/submodule.c
> index 543b1123ae12..b3bb59f06644 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -865,7 +865,8 @@ static void collect_changed_submodules(struct repository *r,
>  		diff_rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
>  		diff_rev.diffopt.format_callback = collect_changed_submodules_cb;
>  		diff_rev.diffopt.format_callback_data = &data;
> -		diff_tree_combined_merge(commit, 1, &diff_rev);
> +		diff_rev.dense_combined_merges = 1;
> +		diff_tree_combined_merge(commit, &diff_rev);
>  	}
>  
>  	reset_revision_walk();

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

end of thread, other threads:[~2020-09-29 18:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 11:31 [PATCH] diff: get rid of redundant 'dense' argument Sergey Organov
2020-09-29 18:54 ` Junio C Hamano

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

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).