git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: kristofferhaugsbakk@fastmail.com
To: git@vger.kernel.org
Cc: Kristoffer Haugsbakk <code@khaugsbakk.name>,
	Denton Liu <liu.denton@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 1/3] range-diff: rename other_arg to log_arg
Date: Thu, 25 Sep 2025 19:07:34 +0200	[thread overview]
Message-ID: <bd037df14f5.1758819879.git.code@khaugsbakk.name> (raw)
In-Reply-To: <v2-cover.1758819879.git.code@khaugsbakk.name>

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

Rename `other_arg` to `log_arg` in `range_diff_options` and
related places.

“Other argument” comes from bd361918 (range-diff: pass through --notes
to `git log`, 2019-11-20) which introduced Git notes handling to
git-range-diff(1) by passing that option on to git-log(1). And that kind
of name might be fine in a local context. However, it was initially
spread among multiple files, and is now[1] part of the
`range_diff_options` struct. It is, prima facie, difficult to guess what
“other” means, especially when just looking at the struct.

But with a little reading we find out that it is used for `--[no-]notes`
and `--diff-merges`, which are both passed on to git-log(1). We should
just rename it to reflect this role; `log_arg` suggests, along with the
`strvec` type, that it is used to pass extra arguments to git-log(1).

† 1: since f1ce6c19 (range-diff: combine all options in a single data
     structure, 2021-02-05)

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    v2 (new):
    
    Add a preliminary commit after discussing the `other_arg` name with
    Junio.
    
      “ Back when it was a random one-shot variable in range-diff, it might
        not have mattered all that much, but now we have it as a proper
        member of the struct, can we give it a name better than 'other_arg"?
    
    Link: https://lore.kernel.org/git/xmqqikharvyl.fsf@gitster.g/
    
    Later:
    
      “ My personal preference is to name arrays singular so that you can
        name its 0th element by saying dog[0], not dogs[0].  "dog[1] and
        dog[2] are friends" not dogs[1] and dogs[2].  An exception is when
        most of the time you use the array as a single unit as a collection,
        passing it around in the call chain, rarely addressing each individual
        element.  I am OK to see such an array called plural (but of course,
        singular names are always fine)..
    
    And I chose singular to personal preference in this context (the use
    etc. for this kind of variable).

 builtin/log.c        |  8 ++++----
 builtin/range-diff.c | 16 ++++++++--------
 range-diff.c         | 10 +++++-----
 range-diff.h         |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 5f552d14c0f..131512ac1af 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1400,13 +1400,13 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		 * can be added later if deemed desirable.
 		 */
 		struct diff_options opts;
-		struct strvec other_arg = STRVEC_INIT;
+		struct strvec log_arg = STRVEC_INIT;
 		struct range_diff_options range_diff_opts = {
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.log_arg = &log_arg
 		};
 
 		repo_diff_setup(the_repository, &opts);
@@ -1414,9 +1414,9 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 		opts.use_color = rev->diffopt.use_color;
 		diff_setup_done(&opts);
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
-		get_notes_args(&other_arg, rev);
+		get_notes_args(&log_arg, rev);
 		show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);
-		strvec_clear(&other_arg);
+		strvec_clear(&log_arg);
 	}
 }
 
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index aafcc99b962..f88b40e3607 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -37,13 +37,13 @@ int cmd_range_diff(int argc,
 		   struct repository *repo UNUSED)
 {
 	struct diff_options diffopt = { NULL };
-	struct strvec other_arg = STRVEC_INIT;
+	struct strvec log_arg = STRVEC_INIT;
 	struct strvec diff_merges_arg = STRVEC_INIT;
 	struct range_diff_options range_diff_opts = {
 		.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
 		.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
 		.diffopt = &diffopt,
-		.other_arg = &other_arg
+		.log_arg = &log_arg
 	};
 	int simple_color = -1, left_only = 0, right_only = 0;
 	struct option range_diff_options[] = {
@@ -52,7 +52,7 @@ int cmd_range_diff(int argc,
 			    N_("percentage by which creation is weighted")),
 		OPT_BOOL(0, "no-dual-color", &simple_color,
 			    N_("use simple diff colors")),
-		OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
+		OPT_PASSTHRU_ARGV(0, "notes", &log_arg,
 				  N_("notes"), N_("passed to 'git log'"),
 				  PARSE_OPT_OPTARG),
 		OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
@@ -92,7 +92,7 @@ int cmd_range_diff(int argc,
 	/* If `--diff-merges` was specified, imply `--merges` */
 	if (diff_merges_arg.nr) {
 		range_diff_opts.include_merges = 1;
-		strvec_pushv(&other_arg, diff_merges_arg.v);
+		strvec_pushv(&log_arg, diff_merges_arg.v);
 	}
 
 	for (i = 0; i < argc; i++)
@@ -124,7 +124,7 @@ int cmd_range_diff(int argc,
 		strbuf_addf(&range1, "%s..%s", argv[0], argv[1]);
 		strbuf_addf(&range2, "%s..%s", argv[0], argv[2]);
 
-		strvec_pushv(&other_arg, argv +
+		strvec_pushv(&log_arg, argv +
 			     (dash_dash < 0 ? 3 : dash_dash));
 	} else if (dash_dash == 2 ||
 		   (dash_dash < 0 && argc > 1 &&
@@ -144,7 +144,7 @@ int cmd_range_diff(int argc,
 		strbuf_addstr(&range1, argv[0]);
 		strbuf_addstr(&range2, argv[1]);
 
-		strvec_pushv(&other_arg, argv +
+		strvec_pushv(&log_arg, argv +
 			     (dash_dash < 0 ? 2 : dash_dash));
 	} else if (dash_dash == 1 ||
 		   (dash_dash < 0 && argc > 0 &&
@@ -175,7 +175,7 @@ int cmd_range_diff(int argc,
 		strbuf_addf(&range1, "%s..%.*s", b, a_len, a);
 		strbuf_addf(&range2, "%.*s..%s", a_len, a, b);
 
-		strvec_pushv(&other_arg, argv +
+		strvec_pushv(&log_arg, argv +
 			     (dash_dash < 0 ? 1 : dash_dash));
 	} else
 		usage_msg_opt(_("need two commit ranges"),
@@ -187,7 +187,7 @@ int cmd_range_diff(int argc,
 	range_diff_opts.right_only = right_only;
 	res = show_range_diff(range1.buf, range2.buf, &range_diff_opts);
 
-	strvec_clear(&other_arg);
+	strvec_clear(&log_arg);
 	strvec_clear(&diff_merges_arg);
 	strbuf_release(&range1);
 	strbuf_release(&range2);
diff --git a/range-diff.c b/range-diff.c
index ca449a07693..57edff40a85 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -39,7 +39,7 @@ struct patch_util {
  * as struct object_id (will need to be free()d).
  */
 static int read_patches(const char *range, struct string_list *list,
-			const struct strvec *other_arg,
+			const struct strvec *log_arg,
 			unsigned int include_merges)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -69,8 +69,8 @@ static int read_patches(const char *range, struct string_list *list,
 	if (!include_merges)
 		strvec_push(&cp.args, "--no-merges");
 	strvec_push(&cp.args, range);
-	if (other_arg)
-		strvec_pushv(&cp.args, other_arg->v);
+	if (log_arg)
+		strvec_pushv(&cp.args, log_arg->v);
 	cp.out = -1;
 	cp.no_stdin = 1;
 	cp.git_cmd = 1;
@@ -594,9 +594,9 @@ int show_range_diff(const char *range1, const char *range2,
 	if (range_diff_opts->left_only && range_diff_opts->right_only)
 		res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only");
 
-	if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg, include_merges))
+	if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges))
 		res = error(_("could not parse log for '%s'"), range1);
-	if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg, include_merges))
+	if (!res && read_patches(range2, &branch2, range_diff_opts->log_arg, include_merges))
 		res = error(_("could not parse log for '%s'"), range2);
 
 	if (!res) {
diff --git a/range-diff.h b/range-diff.h
index 9d39818e349..9b70a80009e 100644
--- a/range-diff.h
+++ b/range-diff.h
@@ -23,7 +23,7 @@ struct range_diff_options {
 	unsigned include_merges:1;
 	size_t max_memory;
 	const struct diff_options *diffopt; /* may be NULL */
-	const struct strvec *other_arg; /* may be NULL */
+	const struct strvec *log_arg; /* may be NULL */
 };
 
 /*
-- 
2.51.0.311.g9b2318464ce


  reply	other threads:[~2025-09-25 17:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22 21:10 [PATCH 0/2] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk
2025-09-22 21:10 ` [PATCH 1/2] revision: add rdiff_other_arg to rev_info kristofferhaugsbakk
2025-09-22 21:58   ` Junio C Hamano
2025-09-23 15:53     ` Kristoffer Haugsbakk
2025-09-23 17:35       ` Junio C Hamano
2025-09-23 17:47         ` Kristoffer Haugsbakk
2025-09-23 21:18           ` Junio C Hamano
2025-09-22 21:10 ` [PATCH 2/2] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk
2025-09-22 22:01   ` Junio C Hamano
2025-09-23 16:26     ` Kristoffer Haugsbakk
2025-09-23 21:20       ` Junio C Hamano
2025-09-25 17:07 ` [PATCH v2 0/3] " kristofferhaugsbakk
2025-09-25 17:07   ` kristofferhaugsbakk [this message]
2025-09-25 17:07   ` [PATCH v2 2/3] revision: add rdiff_log_arg to rev_info kristofferhaugsbakk
2025-09-25 17:07   ` [PATCH v2 3/3] format-patch: handle range-diff on notes correctly for single patches kristofferhaugsbakk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bd037df14f5.1758819879.git.code@khaugsbakk.name \
    --to=kristofferhaugsbakk@fastmail.com \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).