[PATCH 0/2] parse-options: introduce die_for_required_opt() helper

[PATCH 0/2] parse-options: introduce die_for_required_opt() helper

[Siddharth Shrimali]

Many built-in commands in Git manually check for option prerequisites 
(i.e., option X relies on option Y being present) using explicit 
conditional blocks and duplicated error message strings.

This short series comes out of a discussion with Christian about 
localization and code duplication. To address these issues, it 
introduces a centralized API helper that handles simple option 
prerequisites safely.

- Patch 1 introduces the `die_for_required_opt()` helper function 
  inside parse-options.
  
- Patch 2 cleans up `builtin/add.c` as a proof-of-concept by migrating 
  its manual prerequisite checks for '--ignore-missing' and 
  '--pathspec-file-nul' over to the new helper.

If this initial approach looks good, we can later extend the helper 
to handle more complex multi-option dependencies.

Siddharth Shrimali (2):
  parse-options: introduce die_for_required_opt()
  builtin/add: use die_for_required_opt() helper

 builtin/add.c   | 7 +++----
 parse-options.c | 7 +++++++
 parse-options.h | 3 +++
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.54.0

[PATCH 1/2] parse-options: introduce die_for_required_opt()

[Siddharth Shrimali]

Introduce a new helper function die_for_required_opt() to check if a
given option is present without its required prerequisite option.

This provides a centralized API for handling simple option dependencies
(i.e., X requires Y), matching the style of the existing mutual-exclusion
helpers like die_for_incompatible_opt{2,3,4}().

Suggested-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 parse-options.c | 7 +++++++
 parse-options.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/parse-options.c b/parse-options.c
index a676da86f5..e100f9a0c1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1558,3 +1558,10 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
 		break;
 	}
 }
+
+void die_for_required_opt(int opt1, const char *opt1_name,
+			  int opt2, const char *opt2_name)
+{
+	if (opt1 && !opt2)
+		die(_("the option '%s' requires '%s'"), opt1_name, opt2_name);
+}
diff --git a/parse-options.h b/parse-options.h
index 0d1f738f8d..99dc53325d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -460,6 +460,9 @@ static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
 				  0, "");
 }
 
+void die_for_required_opt(int opt1, const char *opt1_name,
+			  int opt2, const char *opt2_name);
+
 /*
  * Use these assertions for callbacks that expect to be called with NONEG and
  * NOARG respectively, and do not otherwise handle the "unset" and "arg"
-- 
2.54.0

[PATCH 2/2] builtin/add: use die_for_required_opt() helper

[Siddharth Shrimali]

Clean up manual option dependency checks by replacing explicit conditional
blocks with the newly introduced die_for_required_opt() helper function.

Specifically, simplify the prerequisite check logic for both
'--ignore-missing' (which requires '--dry-run') and '--pathspec-file-nul'
(which requires '--pathspec-from-file').

Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/add.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..a5c91c6dcf 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -441,8 +441,7 @@ int cmd_add(int argc,
 	if (addremove && take_worktree_changes)
 		die(_("options '%s' and '%s' cannot be used together"), "-A", "-u");
 
-	if (!show_only && ignore_missing)
-		die(_("the option '%s' requires '%s'"), "--ignore-missing", "--dry-run");
+	die_for_required_opt(ignore_missing, "--ignore-missing", show_only, "--dry-run");
 
 	if (chmod_arg && ((chmod_arg[0] != '-' && chmod_arg[0] != '+') ||
 			  chmod_arg[1] != 'x' || chmod_arg[2]))
@@ -462,6 +461,8 @@ int cmd_add(int argc,
 		       PATHSPEC_SYMLINK_LEADING_PATH,
 		       prefix, argv);
 
+	die_for_required_opt(pathspec_file_nul, "--pathspec-file-nul",
+				!!pathspec_from_file, "--pathspec-from-file");
 	if (pathspec_from_file) {
 		if (pathspec.nr)
 			die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
@@ -470,8 +471,6 @@ int cmd_add(int argc,
 				    PATHSPEC_PREFER_FULL |
 				    PATHSPEC_SYMLINK_LEADING_PATH,
 				    prefix, pathspec_from_file, pathspec_file_nul);
-	} else if (pathspec_file_nul) {
-		die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
 	}
 
 	if (require_pathspec && pathspec.nr == 0) {
-- 
2.54.0

Re: [PATCH 1/2] parse-options: introduce die_for_required_opt()

[Jean-Noël AVILA]

On Wednesday, 3 June 2026 13:10:43 CEST Siddharth Shrimali wrote:
> Introduce a new helper function die_for_required_opt() to check if a
> given option is present without its required prerequisite option.
> 
> This provides a centralized API for handling simple option dependencies
> (i.e., X requires Y), matching the style of the existing mutual-exclusion
> helpers like die_for_incompatible_opt{2,3,4}().
> 
> Suggested-by: Christian Couder <christian.couder@gmail.com>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>  parse-options.c | 7 +++++++
>  parse-options.h | 3 +++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/parse-options.c b/parse-options.c
> index a676da86f5..e100f9a0c1 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1558,3 +1558,10 @@ void die_for_incompatible_opt4(int opt1, const char
> *opt1_name, break;
>  	}
>  }
> +
> +void die_for_required_opt(int opt1, const char *opt1_name,
> +			  int opt2, const char *opt2_name)

Hello,

First thanks for trying to uniformize/simplify option checking. The 
translators will be happy.

To me, "die_for_required_opt" is a misnomer as the function does not die for 
an existing "required" condition, unlike the other functions such as 
die_for_incompatible_opt<n>.

The names of the parameters do not indicate that the test is not symmetrical 
(not failing on XOR).

Maybe something like "die_for_missing_opt(int tested_opt, const char 
*tested_opt_name, int required_opt, const char *required_opt_name)

would make it more understandable.