All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH 4/8] checkout: extract option validation and pathspec helpers
Date: Fri, 28 Aug 2026 15:52:02 -0700	[thread overview]
Message-ID: <20260828225206.310500-5-gitster@pobox.com> (raw)
In-Reply-To: <20260828225206.310500-1-gitster@pobox.com>

The checkout_main() function handles initialization, option parsing,
option validation, and pathspec resolution.

Extract option validation and pathspec handling logic into static
helper functions:

  - init_checkout_opts() initializes default checkout options and
    repo settings.

  - validate_path_options() validates patch context and
    index/worktree flags.

  - prepare_common_options() prepares progress, merge, and force
    options.

  - parse_pathspec_from_file_options() parses and validates
    pathspecs read from a file.

Call these helpers from checkout_main().

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/checkout.c | 171 +++++++++++++++++++++++++--------------------
 1 file changed, 96 insertions(+), 75 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e3d23256e2..8d567def7e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1846,6 +1846,98 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,
 	return newopts;
 }
 
+static void init_checkout_opts(struct checkout_opts *opts, const char *prefix)
+{
+	opts->overwrite_ignore = 1;
+	opts->prefix = prefix;
+	opts->show_progress = -1;
+
+	repo_config(the_repository, git_checkout_config, opts);
+	if (the_repository->gitdir) {
+		prepare_repo_settings(the_repository);
+		the_repository->settings.command_requires_full_index = 0;
+	}
+
+	opts->track = BRANCH_TRACK_UNSPECIFIED;
+}
+
+static void validate_path_options(struct checkout_opts *opts)
+{
+	if (opts->patch_context < -1)
+		die(_("'%s' cannot be negative"), "--unified");
+	if (opts->patch_interhunk_context < -1)
+		die(_("'%s' cannot be negative"), "--inter-hunk-context");
+
+	if (!opts->patch_mode) {
+		if (opts->patch_context != -1)
+			die(_("the option '%s' requires '%s'"), "--unified", "--patch");
+		if (opts->patch_interhunk_context != -1)
+			die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch");
+		if (!opts->auto_advance)
+			die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch");
+	}
+
+	if (opts->overlay_mode == 1 && opts->patch_mode)
+		die(_("options '%s' and '%s' cannot be used together"), "-p", "--overlay");
+
+	if (opts->checkout_index >= 0 || opts->checkout_worktree >= 0) {
+		if (opts->checkout_index < 0)
+			opts->checkout_index = 0;
+		if (opts->checkout_worktree < 0)
+			opts->checkout_worktree = 0;
+	} else {
+		if (opts->checkout_index < 0)
+			opts->checkout_index = -opts->checkout_index - 1;
+		if (opts->checkout_worktree < 0)
+			opts->checkout_worktree = -opts->checkout_worktree - 1;
+	}
+	if (opts->checkout_index < 0 || opts->checkout_worktree < 0)
+		BUG("these flags should be non-negative by now");
+}
+
+static void prepare_common_options(struct checkout_opts *opts)
+{
+	if (opts->show_progress < 0) {
+		if (opts->quiet)
+			opts->show_progress = 0;
+		else
+			opts->show_progress = isatty(2);
+	}
+
+	/* --conflicts implies --merge */
+	if (opts->merge == -1)
+		opts->merge = opts->conflict_style >= 0;
+
+	if (opts->force) {
+		opts->discard_changes = 1;
+		opts->ignore_unmerged_opt = "--force";
+		opts->ignore_unmerged = 1;
+	}
+}
+
+static void parse_pathspec_from_file_options(struct checkout_opts *opts,
+					     const char *prefix)
+{
+	if (opts->pathspec_from_file) {
+		if (opts->pathspec.nr)
+			die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
+
+		if (opts->force_detach)
+			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--detach");
+
+		if (opts->patch_mode)
+			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
+
+		parse_pathspec_file(&opts->pathspec, 0,
+				    0,
+				    prefix, opts->pathspec_from_file, opts->pathspec_file_nul);
+	} else if (opts->pathspec_file_nul) {
+		die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
+	}
+
+	opts->pathspec.recursive = 1;
+}
+
 static int checkout_main(int argc, const char **argv, const char *prefix,
 			 struct checkout_opts *opts, struct option *options,
 			 enum checkout_command which_command)
@@ -1887,17 +1979,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 		BUG("no such checkout variant %d", which_command);
 	}
 
-	opts->overwrite_ignore = 1;
-	opts->prefix = prefix;
-	opts->show_progress = -1;
-
-	repo_config(the_repository, git_checkout_config, opts);
-	if (the_repository->gitdir) {
-		prepare_repo_settings(the_repository);
-		the_repository->settings.command_requires_full_index = 0;
-	}
-
-	opts->track = BRANCH_TRACK_UNSPECIFIED;
+	init_checkout_opts(opts, prefix);
 
 	if (!opts->accept_pathspec && !opts->accept_ref)
 		BUG("make up your mind, you need to take _something_");
@@ -1907,57 +1989,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 	argc = parse_options(argc, argv, prefix, options,
 			     usagestr, parseopt_flags);
 
-	if (opts->patch_context < -1)
-		die(_("'%s' cannot be negative"), "--unified");
-	if (opts->patch_interhunk_context < -1)
-		die(_("'%s' cannot be negative"), "--inter-hunk-context");
-
-	if (!opts->patch_mode) {
-		if (opts->patch_context != -1)
-			die(_("the option '%s' requires '%s'"), "--unified", "--patch");
-		if (opts->patch_interhunk_context != -1)
-			die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch");
-		if (!opts->auto_advance)
-			die(_("the option '%s' requires '%s'"), "--no-auto-advance", "--patch");
-	}
-
-	if (opts->show_progress < 0) {
-		if (opts->quiet)
-			opts->show_progress = 0;
-		else
-			opts->show_progress = isatty(2);
-	}
-
-	/* --conflicts implies --merge */
-	if (opts->merge == -1)
-		opts->merge = opts->conflict_style >= 0;
-
-	if (opts->force) {
-		opts->discard_changes = 1;
-		opts->ignore_unmerged_opt = "--force";
-		opts->ignore_unmerged = 1;
-	}
+	validate_path_options(opts);
+	prepare_common_options(opts);
 
 	if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1)
 		die(_("options '-%c', '-%c', and '%s' cannot be used together"),
 			cb_option, toupper(cb_option), "--orphan");
 
-	if (opts->overlay_mode == 1 && opts->patch_mode)
-		die(_("options '%s' and '%s' cannot be used together"), "-p", "--overlay");
-
-	if (opts->checkout_index >= 0 || opts->checkout_worktree >= 0) {
-		if (opts->checkout_index < 0)
-			opts->checkout_index = 0;
-		if (opts->checkout_worktree < 0)
-			opts->checkout_worktree = 0;
-	} else {
-		if (opts->checkout_index < 0)
-			opts->checkout_index = -opts->checkout_index - 1;
-		if (opts->checkout_worktree < 0)
-			opts->checkout_worktree = -opts->checkout_worktree - 1;
-	}
-	if (opts->checkout_index < 0 || opts->checkout_worktree < 0)
-		BUG("these flags should be non-negative by now");
 	/*
 	 * convenient shortcut: "git restore --staged [--worktree]" equals
 	 * "git restore --staged [--worktree] --source HEAD"
@@ -2048,24 +2086,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 			    argv[0]);
 	}
 
-	if (opts->pathspec_from_file) {
-		if (opts->pathspec.nr)
-			die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
-
-		if (opts->force_detach)
-			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--detach");
-
-		if (opts->patch_mode)
-			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--patch");
-
-		parse_pathspec_file(&opts->pathspec, 0,
-				    0,
-				    prefix, opts->pathspec_from_file, opts->pathspec_file_nul);
-	} else if (opts->pathspec_file_nul) {
-		die(_("the option '%s' requires '%s'"), "--pathspec-file-nul", "--pathspec-from-file");
-	}
-
-	opts->pathspec.recursive = 1;
+	parse_pathspec_from_file_options(opts, prefix);
 
 	if (!opts->pathspec.nr) {
 		if (opts->accept_pathspec && !opts->empty_pathspec_ok &&
-- 
2.55.0-884-g76cf8659c2


  parent reply	other threads:[~2026-08-28 22:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27 18:29 [PATCH] builtin: replace the_repository parameter in is_bare_repository() Hardik Kumar
2026-08-27 19:09 ` Junio C Hamano
2026-08-27 19:51   ` Junio C Hamano
2026-08-27 20:09     ` Hardik Kumar
2026-08-27 20:28       ` Junio C Hamano
2026-08-27 21:12         ` Ben Knoble
2026-08-27 21:39           ` Junio C Hamano
2026-08-28 11:41             ` D. Ben Knoble
2026-08-28 22:51               ` Junio C Hamano
2026-08-28 22:51                 ` [PATCH 0/8] More sensible checkout/switch/restore code refactoring Junio C Hamano
2026-08-28 22:51                   ` [PATCH 1/8] checkout: pass cb_option explicitly to branch name parsers Junio C Hamano
2026-08-28 22:52                   ` [PATCH 2/8] checkout: validate new branch name in checkout_branch() Junio C Hamano
2026-08-28 22:52                   ` [PATCH 3/8] checkout: validate stage and merge option compatibility in checkout_paths() Junio C Hamano
2026-08-28 22:52                   ` Junio C Hamano [this message]
2026-08-28 22:52                   ` [PATCH 5/8] checkout: extract branch setup and tracking helpers Junio C Hamano
2026-08-28 22:52                   ` [PATCH 6/8] checkout: restructure switch, restore, and checkout entrypoints Junio C Hamano
2026-08-28 22:52                   ` [PATCH 7/8] checkout: wrap overly long lines Junio C Hamano
2026-08-28 22:55                     ` Junio C Hamano
2026-08-29  2:06                       ` Junio C Hamano
2026-08-28 22:52                   ` [PATCH 8/8] checkout: move post_checkout_hook() to checkout.c Junio C Hamano
2026-08-28 22:57                     ` Junio C Hamano
2026-08-29  2:05                       ` Junio C Hamano
2026-08-29 13:24                 ` [PATCH] builtin: replace the_repository parameter in is_bare_repository() D. Ben Knoble
2026-08-27 21:35         ` [PATCH] do not pass "repo" to builtin commmand implementations Junio C Hamano
2026-08-28  9:05           ` Hardik Kumar
2026-08-28 20:59             ` Junio C Hamano
2026-08-28  4:01         ` [PATCH] builtin: replace the_repository parameter in is_bare_repository() Hardik Kumar
2026-08-27 19:56   ` Hardik Kumar

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=20260828225206.310500-5-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.