[Outreachy PATCH] range-diff: use the repository from the diff options

[Outreachy PATCH] range-diff: use the repository from the diff options

[Okhuomon Ajayi]

output_pair_header() and patch_diff() already receive the diff
options, but use the_repository and the_hash_algo. Use the
repository from diffopt instead, and pass only the hash algorithm
to get_filespec(), as that is all it needs.

read_patches(), output() and is_range_diff_range() still use
the_repository, so USE_THE_REPOSITORY_VARIABLE stays.

Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
---
 range-diff.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 8e2dd2eb19..3f56a4f71a 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -418,7 +418,7 @@ static void output_pair_header(struct diff_options *diffopt,
 
 	if (!dashes->len)
 		strbuf_addchars(dashes, '-',
-				strlen(repo_find_unique_abbrev(the_repository, oid, abbrev)));
+				strlen(repo_find_unique_abbrev(diffopt->repo, oid, abbrev)));
 
 	if (!b_util) {
 		color = color_old;
@@ -440,7 +440,7 @@ static void output_pair_header(struct diff_options *diffopt,
 		strbuf_addf(buf, "%*s:  %s ", patch_no_width, "-", dashes->buf);
 	else
 		strbuf_addf(buf, "%*d:  %s ", patch_no_width, a_util->i + 1,
-			    repo_find_unique_abbrev(the_repository, &a_util->oid, abbrev));
+			    repo_find_unique_abbrev(diffopt->repo, &a_util->oid, abbrev));
 
 	if (status == '!')
 		strbuf_addf(buf, "%s%s", color_reset, color);
@@ -452,9 +452,9 @@ static void output_pair_header(struct diff_options *diffopt,
 		strbuf_addf(buf, " %*s:  %s", patch_no_width, "-", dashes->buf);
 	else
 		strbuf_addf(buf, " %*d:  %s", patch_no_width, b_util->i + 1,
-			    repo_find_unique_abbrev(the_repository, &b_util->oid, abbrev));
+			    repo_find_unique_abbrev(diffopt->repo, &b_util->oid, abbrev));
 
-	commit = lookup_commit_reference(the_repository, oid);
+	commit = lookup_commit_reference(diffopt->repo, oid);
 	if (commit) {
 		if (status == '!')
 			strbuf_addf(buf, "%s%s", color_reset, color);
@@ -474,11 +474,12 @@ static struct userdiff_driver section_headers = {
 	},
 };
 
-static struct diff_filespec *get_filespec(const char *name, const char *p)
+static struct diff_filespec *get_filespec(const struct git_hash_algo *algop,
+					  const char *name, const char *p)
 {
 	struct diff_filespec *spec = alloc_filespec(name);
 
-	fill_filespec(spec, null_oid(the_hash_algo), 0, 0100644);
+	fill_filespec(spec, null_oid(algop), 0, 0100644);
 	spec->data = (char *)p;
 	spec->size = strlen(p);
 	spec->should_munmap = 0;
@@ -492,7 +493,8 @@ static void patch_diff(const char *a, const char *b,
 		       struct diff_options *diffopt)
 {
 	diff_queue(&diff_queued_diff,
-		   get_filespec("a", a), get_filespec("b", b));
+		   get_filespec(diffopt->repo->hash_algo, "a", a),
+		   get_filespec(diffopt->repo->hash_algo, "b", b));
 
 	diffcore_std(diffopt);
 	diff_flush(diffopt);

base-commit: 8d1b215c01512b9658d4525c54ff9b9245f9a7e8
-- 
2.56.0

Re: [Outreachy PATCH] range-diff: use the repository from the diff options

[Pablo Sabater]

On Wed Oct 28, 2026 at 9:51 PM WET, Okhuomon Ajayi wrote:

Hi!

I think that the title, "range-diff: use the repository from the diff
options" can be confusing for someone that doesn't know that diffopt has
a repo field (as it happened to me when reading this).

Also, when talking about variables in our code, using their names makes
it easier to know which code is referred to.

Does this work for you as title?:

range-diff: use diffopt->repo instead of the_repository

> output_pair_header() and patch_diff() already receive the diff
> options, but use the_repository and the_hash_algo. Use the

This first sentence reads a bit strange. But apart from that, I miss a
reason about why we want this [1].

> repository from diffopt instead, and pass only the hash algorithm
> to get_filespec(), as that is all it needs.

"and pass only" reads as if now get_filespec() only receives the hash
algo, but it receives the hash algo on top of what it was already
receiving.

Also, saying that this is related to the_repository replacement because
the_hash_algo comes from the_repository would be nice.

>
> read_patches(), output() and is_range_diff_range() still use
> the_repository, so USE_THE_REPOSITORY_VARIABLE stays.

Nice to know. But I think that it would read better [2] if you used the
verb at the start, like:

Keep USE_THE_REPOSITORY_VARIABLE because read_patches(), output() and
is_range_diff_range() still use the_repository.

I recommend reading Documentation/SubmittingPatches for this kind of
things. I left at the bottom two links.

>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
>  range-diff.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/range-diff.c b/range-diff.c
> index 8e2dd2eb19..3f56a4f71a 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -418,7 +418,7 @@ static void output_pair_header(struct diff_options *diffopt,
>  
>  	if (!dashes->len)
>  		strbuf_addchars(dashes, '-',
> -				strlen(repo_find_unique_abbrev(the_repository, oid, abbrev)));
> +				strlen(repo_find_unique_abbrev(diffopt->repo, oid, abbrev)));
>  
>  	if (!b_util) {
>  		color = color_old;
> @@ -440,7 +440,7 @@ static void output_pair_header(struct diff_options *diffopt,
>  		strbuf_addf(buf, "%*s:  %s ", patch_no_width, "-", dashes->buf);
>  	else
>  		strbuf_addf(buf, "%*d:  %s ", patch_no_width, a_util->i + 1,
> -			    repo_find_unique_abbrev(the_repository, &a_util->oid, abbrev));
> +			    repo_find_unique_abbrev(diffopt->repo, &a_util->oid, abbrev));
>  
>  	if (status == '!')
>  		strbuf_addf(buf, "%s%s", color_reset, color);
> @@ -452,9 +452,9 @@ static void output_pair_header(struct diff_options *diffopt,
>  		strbuf_addf(buf, " %*s:  %s", patch_no_width, "-", dashes->buf);
>  	else
>  		strbuf_addf(buf, " %*d:  %s", patch_no_width, b_util->i + 1,
> -			    repo_find_unique_abbrev(the_repository, &b_util->oid, abbrev));
> +			    repo_find_unique_abbrev(diffopt->repo, &b_util->oid, abbrev));
>  
> -	commit = lookup_commit_reference(the_repository, oid);
> +	commit = lookup_commit_reference(diffopt->repo, oid);
>  	if (commit) {
>  		if (status == '!')
>  			strbuf_addf(buf, "%s%s", color_reset, color);
> @@ -474,11 +474,12 @@ static struct userdiff_driver section_headers = {
>  	},
>  };
>  
> -static struct diff_filespec *get_filespec(const char *name, const char *p)
> +static struct diff_filespec *get_filespec(const struct git_hash_algo *algop,
> +					  const char *name, const char *p)
>  {
>  	struct diff_filespec *spec = alloc_filespec(name);
>  
> -	fill_filespec(spec, null_oid(the_hash_algo), 0, 0100644);
> +	fill_filespec(spec, null_oid(algop), 0, 0100644);
>  	spec->data = (char *)p;
>  	spec->size = strlen(p);
>  	spec->should_munmap = 0;
> @@ -492,7 +493,8 @@ static void patch_diff(const char *a, const char *b,
>  		       struct diff_options *diffopt)
>  {
>  	diff_queue(&diff_queued_diff,
> -		   get_filespec("a", a), get_filespec("b", b));
> +		   get_filespec(diffopt->repo->hash_algo, "a", a),
> +		   get_filespec(diffopt->repo->hash_algo, "b", b));
>  
>  	diffcore_std(diffopt);
>  	diff_flush(diffopt);
>
> base-commit: 8d1b215c01512b9658d4525c54ff9b9245f9a7e8

The code looks good to me.
Passes make DEVELOPER=1 and git clang-format --diff HEAD~

[1]: https://git-scm.com/docs/SubmittingPatches#meaningful-message
[2]: https://git-scm.com/docs/SubmittingPatches#imperative-mood

Good job,
Pablo

[Outreachy PATCH v2] range-diff: use diffopt->repo instead of the_repository

[Okhuomon Ajayi]

output_pair_header() and patch_diff() receive a diffopt whose repo
field names the repository to work on, yet they abbreviate object
names and look up commits in the_repository. get_filespec(), called
from patch_diff(), also uses the_hash_algo, which is the hash
algorithm of the_repository. If a caller set up diffopt for another
repository, these functions would still read from the global one.

Use diffopt->repo in output_pair_header(), and add a hash algorithm
parameter to get_filespec() so that patch_diff() can pass
diffopt->repo->hash_algo.

Keep USE_THE_REPOSITORY_VARIABLE because read_patches(), output() and
is_range_diff_range() still use the_repository.

Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
---
Changes since v1:
- Reworded the commit message as suggested by Pablo. No code was
  changed.

Thanks, Pablo, for the review.

 range-diff.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/range-diff.c b/range-diff.c
index 8e2dd2eb19..3f56a4f71a 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -418,7 +418,7 @@ static void output_pair_header(struct diff_options *diffopt,
 
 	if (!dashes->len)
 		strbuf_addchars(dashes, '-',
-				strlen(repo_find_unique_abbrev(the_repository, oid, abbrev)));
+				strlen(repo_find_unique_abbrev(diffopt->repo, oid, abbrev)));
 
 	if (!b_util) {
 		color = color_old;
@@ -440,7 +440,7 @@ static void output_pair_header(struct diff_options *diffopt,
 		strbuf_addf(buf, "%*s:  %s ", patch_no_width, "-", dashes->buf);
 	else
 		strbuf_addf(buf, "%*d:  %s ", patch_no_width, a_util->i + 1,
-			    repo_find_unique_abbrev(the_repository, &a_util->oid, abbrev));
+			    repo_find_unique_abbrev(diffopt->repo, &a_util->oid, abbrev));
 
 	if (status == '!')
 		strbuf_addf(buf, "%s%s", color_reset, color);
@@ -452,9 +452,9 @@ static void output_pair_header(struct diff_options *diffopt,
 		strbuf_addf(buf, " %*s:  %s", patch_no_width, "-", dashes->buf);
 	else
 		strbuf_addf(buf, " %*d:  %s", patch_no_width, b_util->i + 1,
-			    repo_find_unique_abbrev(the_repository, &b_util->oid, abbrev));
+			    repo_find_unique_abbrev(diffopt->repo, &b_util->oid, abbrev));
 
-	commit = lookup_commit_reference(the_repository, oid);
+	commit = lookup_commit_reference(diffopt->repo, oid);
 	if (commit) {
 		if (status == '!')
 			strbuf_addf(buf, "%s%s", color_reset, color);
@@ -474,11 +474,12 @@ static struct userdiff_driver section_headers = {
 	},
 };
 
-static struct diff_filespec *get_filespec(const char *name, const char *p)
+static struct diff_filespec *get_filespec(const struct git_hash_algo *algop,
+					  const char *name, const char *p)
 {
 	struct diff_filespec *spec = alloc_filespec(name);
 
-	fill_filespec(spec, null_oid(the_hash_algo), 0, 0100644);
+	fill_filespec(spec, null_oid(algop), 0, 0100644);
 	spec->data = (char *)p;
 	spec->size = strlen(p);
 	spec->should_munmap = 0;
@@ -492,7 +493,8 @@ static void patch_diff(const char *a, const char *b,
 		       struct diff_options *diffopt)
 {
 	diff_queue(&diff_queued_diff,
-		   get_filespec("a", a), get_filespec("b", b));
+		   get_filespec(diffopt->repo->hash_algo, "a", a),
+		   get_filespec(diffopt->repo->hash_algo, "b", b));
 
 	diffcore_std(diffopt);
 	diff_flush(diffopt);

base-commit: 8d1b215c01512b9658d4525c54ff9b9245f9a7e8
-- 
2.56.0

Re: [Outreachy PATCH v2] range-diff: use diffopt->repo instead of the_repository

[Junio C Hamano]

Okhuomon Ajayi <okhuomonajayi54@gmail.com> writes:

> output_pair_header() and patch_diff() receive a diffopt whose repo
> field names the repository to work on, yet they abbreviate object
> names and look up commits in the_repository. get_filespec(), called
> from patch_diff(), also uses the_hash_algo, which is the hash
> algorithm of the_repository. If a caller set up diffopt for another
> repository, these functions would still read from the global one.
>
> Use diffopt->repo in output_pair_header(), and add a hash algorithm
> parameter to get_filespec() so that patch_diff() can pass
> diffopt->repo->hash_algo.
>
> Keep USE_THE_REPOSITORY_VARIABLE because read_patches(), output() and
> is_range_diff_range() still use the_repository.
>
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---
> Changes since v1:
> - Reworded the commit message as suggested by Pablo. No code was
>   changed.
>
> Thanks, Pablo, for the review.

OK, well written.  Except for the "In-reply-to:" header that should
point at your earlier iteration of the patch (not a review message
your earlier iteration received), everything I see here looks good.

Will queue.