From: K Jayatheerth <jayatheerthkulkarni2005@gmail•com>
To: git@vger•kernel.org
Cc: jayatheerthkulkarni2005@gmail•com
Subject: [PATCH] Make pull.c match the structural conventions
Date: Fri, 12 Dec 2025 07:39:30 +0530 [thread overview]
Message-ID: <20251212020930.11654-1-jayatheerthkulkarni2005@gmail.com> (raw)
The builtin sources follow a predictable structure, and pull.c departs
from that pattern by arranging its option table in a way that disrupts
the expected flow of the file. The irregular placement makes the file
harder to read, breaks the visual rhythm shared by other builtins, and
forces readers to jump around to understand how options are handled.
The lack of consistency makes pull.c feel like an outlier rather than
a peer alongside the other commands.
A consistent layout helps readers rely on established mental models,
so bringing pull.c into alignment improves clarity and makes the file
easier to navigate and maintain.
Pull.c, become structured like the other builtin/*.c files, keeping the
option definitions where the reader naturally expects them and restoring
the uniformity of the builtin command layout.
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail•com>
---
I was going through pull.c for a different bug hunt and found this out.
I thought there must be some issue pulling the options table locally to main but
git compiles and also passes the tests and also the CI/CD,
so I can't say if this part was intentional or not.
builtin/pull.c | 285 ++++++++++++++++++++++++-------------------------
1 file changed, 142 insertions(+), 143 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 5ebd529620..97814edbc5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -119,148 +119,6 @@ static int opt_show_forced_updates = -1;
static const char *set_upstream;
static struct strvec opt_fetch = STRVEC_INIT;
-static struct option pull_options[] = {
- /* Shared options */
- OPT__VERBOSITY(&opt_verbosity),
- OPT_PASSTHRU(0, "progress", &opt_progress, NULL,
- N_("force progress reporting"),
- PARSE_OPT_NOARG),
- OPT_CALLBACK_F(0, "recurse-submodules",
- &recurse_submodules_cli, N_("on-demand"),
- N_("control for recursive fetching of submodules"),
- PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
-
- /* Options passed to git-merge or git-rebase */
- OPT_GROUP(N_("Options related to merging")),
- OPT_CALLBACK_F('r', "rebase", &opt_rebase,
- "(false|true|merges|interactive)",
- N_("incorporate changes by rebasing rather than merging"),
- PARSE_OPT_OPTARG, parse_opt_rebase),
- OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
- N_("do not show a diffstat at the end of the merge"),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG),
- OPT_PASSTHRU(0, "stat", &opt_diffstat, NULL,
- N_("show a diffstat at the end of the merge"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "summary", &opt_diffstat, NULL,
- N_("(synonym to --stat)"),
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN),
- OPT_PASSTHRU(0, "compact-summary", &opt_diffstat, NULL,
- N_("show a compact-summary at the end of the merge"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "log", &opt_log, N_("n"),
- N_("add (at most <n>) entries from shortlog to merge commit message"),
- PARSE_OPT_OPTARG),
- OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
- N_("add a Signed-off-by trailer"),
- PARSE_OPT_OPTARG),
- OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
- N_("create a single commit instead of doing a merge"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "commit", &opt_commit, NULL,
- N_("perform a commit if the merge succeeds (default)"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "edit", &opt_edit, NULL,
- N_("edit message before committing"),
- PARSE_OPT_NOARG),
- OPT_CLEANUP(&cleanup_arg),
- OPT_PASSTHRU(0, "ff", &opt_ff, NULL,
- N_("allow fast-forward"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL,
- N_("abort if fast-forward is not possible"),
- PARSE_OPT_NOARG | PARSE_OPT_NONEG),
- OPT_PASSTHRU(0, "verify", &opt_verify, NULL,
- N_("control use of pre-merge-commit and commit-msg hooks"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL,
- N_("verify that the named commit has a valid GPG signature"),
- PARSE_OPT_NOARG),
- OPT_BOOL(0, "autostash", &opt_autostash,
- N_("automatically stash/stash pop before and after")),
- OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"),
- N_("merge strategy to use"),
- 0),
- OPT_PASSTHRU_ARGV('X', "strategy-option", &opt_strategy_opts,
- N_("option=value"),
- N_("option for selected merge strategy"),
- 0),
- OPT_PASSTHRU('S', "gpg-sign", &opt_gpg_sign, N_("key-id"),
- N_("GPG sign commit"),
- PARSE_OPT_OPTARG),
- OPT_SET_INT(0, "allow-unrelated-histories",
- &opt_allow_unrelated_histories,
- N_("allow merging unrelated histories"), 1),
-
- /* Options passed to git-fetch */
- OPT_GROUP(N_("Options related to fetching")),
- OPT_PASSTHRU(0, "all", &opt_all, NULL,
- N_("fetch from all remotes"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU('a', "append", &opt_append, NULL,
- N_("append to .git/FETCH_HEAD instead of overwriting"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "upload-pack", &opt_upload_pack, N_("path"),
- N_("path to upload pack on remote end"),
- 0),
- OPT__FORCE(&opt_force, N_("force overwrite of local branch"), 0),
- OPT_PASSTHRU('t', "tags", &opt_tags, NULL,
- N_("fetch all tags and associated objects"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU('p', "prune", &opt_prune, NULL,
- N_("prune remote-tracking branches no longer on remote"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU('j', "jobs", &max_children, N_("n"),
- N_("number of submodules pulled in parallel"),
- PARSE_OPT_OPTARG),
- OPT_BOOL(0, "dry-run", &opt_dry_run,
- N_("dry run")),
- OPT_PASSTHRU('k', "keep", &opt_keep, NULL,
- N_("keep downloaded pack"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "depth", &opt_depth, N_("depth"),
- N_("deepen history of shallow clone"),
- 0),
- OPT_PASSTHRU_ARGV(0, "shallow-since", &opt_fetch, N_("time"),
- N_("deepen history of shallow repository based on time"),
- 0),
- OPT_PASSTHRU_ARGV(0, "shallow-exclude", &opt_fetch, N_("ref"),
- N_("deepen history of shallow clone, excluding ref"),
- 0),
- OPT_PASSTHRU_ARGV(0, "deepen", &opt_fetch, N_("n"),
- N_("deepen history of shallow clone"),
- 0),
- OPT_PASSTHRU(0, "unshallow", &opt_unshallow, NULL,
- N_("convert to a complete repository"),
- PARSE_OPT_NONEG | PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "update-shallow", &opt_update_shallow, NULL,
- N_("accept refs that update .git/shallow"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU(0, "refmap", &opt_refmap, N_("refmap"),
- N_("specify fetch refmap"),
- PARSE_OPT_NONEG),
- OPT_PASSTHRU_ARGV('o', "server-option", &opt_fetch,
- N_("server-specific"),
- N_("option to transmit"),
- 0),
- OPT_PASSTHRU('4', "ipv4", &opt_ipv4, NULL,
- N_("use IPv4 addresses only"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU('6', "ipv6", &opt_ipv6, NULL,
- N_("use IPv6 addresses only"),
- PARSE_OPT_NOARG),
- OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
- N_("report that we have only objects reachable from this object"),
- 0),
- OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
- N_("check for forced-updates on all updated branches")),
- OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
- N_("set upstream for git pull/fetch"),
- PARSE_OPT_NOARG),
-
- OPT_END()
-};
-
/**
* Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
*/
@@ -1008,7 +866,148 @@ int cmd_pull(int argc,
int can_ff;
int divergent;
int ret;
-
+ static struct option pull_options[] = {
+ /* Shared options */
+ OPT__VERBOSITY(&opt_verbosity),
+ OPT_PASSTHRU(0, "progress", &opt_progress, NULL,
+ N_("force progress reporting"),
+ PARSE_OPT_NOARG),
+ OPT_CALLBACK_F(0, "recurse-submodules",
+ &recurse_submodules_cli, N_("on-demand"),
+ N_("control for recursive fetching of submodules"),
+ PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
+
+ /* Options passed to git-merge or git-rebase */
+ OPT_GROUP(N_("Options related to merging")),
+ OPT_CALLBACK_F('r', "rebase", &opt_rebase,
+ "(false|true|merges|interactive)",
+ N_("incorporate changes by rebasing rather than merging"),
+ PARSE_OPT_OPTARG, parse_opt_rebase),
+ OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
+ N_("do not show a diffstat at the end of the merge"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+ OPT_PASSTHRU(0, "stat", &opt_diffstat, NULL,
+ N_("show a diffstat at the end of the merge"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "summary", &opt_diffstat, NULL,
+ N_("(synonym to --stat)"),
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN),
+ OPT_PASSTHRU(0, "compact-summary", &opt_diffstat, NULL,
+ N_("show a compact-summary at the end of the merge"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "log", &opt_log, N_("n"),
+ N_("add (at most <n>) entries from shortlog to merge commit message"),
+ PARSE_OPT_OPTARG),
+ OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
+ N_("add a Signed-off-by trailer"),
+ PARSE_OPT_OPTARG),
+ OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
+ N_("create a single commit instead of doing a merge"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "commit", &opt_commit, NULL,
+ N_("perform a commit if the merge succeeds (default)"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "edit", &opt_edit, NULL,
+ N_("edit message before committing"),
+ PARSE_OPT_NOARG),
+ OPT_CLEANUP(&cleanup_arg),
+ OPT_PASSTHRU(0, "ff", &opt_ff, NULL,
+ N_("allow fast-forward"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL,
+ N_("abort if fast-forward is not possible"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG),
+ OPT_PASSTHRU(0, "verify", &opt_verify, NULL,
+ N_("control use of pre-merge-commit and commit-msg hooks"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL,
+ N_("verify that the named commit has a valid GPG signature"),
+ PARSE_OPT_NOARG),
+ OPT_BOOL(0, "autostash", &opt_autostash,
+ N_("automatically stash/stash pop before and after")),
+ OPT_PASSTHRU_ARGV('s', "strategy", &opt_strategies, N_("strategy"),
+ N_("merge strategy to use"),
+ 0),
+ OPT_PASSTHRU_ARGV('X', "strategy-option", &opt_strategy_opts,
+ N_("option=value"),
+ N_("option for selected merge strategy"),
+ 0),
+ OPT_PASSTHRU('S', "gpg-sign", &opt_gpg_sign, N_("key-id"),
+ N_("GPG sign commit"),
+ PARSE_OPT_OPTARG),
+ OPT_SET_INT(0, "allow-unrelated-histories",
+ &opt_allow_unrelated_histories,
+ N_("allow merging unrelated histories"), 1),
+
+ /* Options passed to git-fetch */
+ OPT_GROUP(N_("Options related to fetching")),
+ OPT_PASSTHRU(0, "all", &opt_all, NULL,
+ N_("fetch from all remotes"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU('a', "append", &opt_append, NULL,
+ N_("append to .git/FETCH_HEAD instead of overwriting"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "upload-pack", &opt_upload_pack, N_("path"),
+ N_("path to upload pack on remote end"),
+ 0),
+ OPT__FORCE(&opt_force, N_("force overwrite of local branch"), 0),
+ OPT_PASSTHRU('t', "tags", &opt_tags, NULL,
+ N_("fetch all tags and associated objects"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU('p', "prune", &opt_prune, NULL,
+ N_("prune remote-tracking branches no longer on remote"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU('j', "jobs", &max_children, N_("n"),
+ N_("number of submodules pulled in parallel"),
+ PARSE_OPT_OPTARG),
+ OPT_BOOL(0, "dry-run", &opt_dry_run,
+ N_("dry run")),
+ OPT_PASSTHRU('k', "keep", &opt_keep, NULL,
+ N_("keep downloaded pack"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "depth", &opt_depth, N_("depth"),
+ N_("deepen history of shallow clone"),
+ 0),
+ OPT_PASSTHRU_ARGV(0, "shallow-since", &opt_fetch, N_("time"),
+ N_("deepen history of shallow repository based on time"),
+ 0),
+ OPT_PASSTHRU_ARGV(0, "shallow-exclude", &opt_fetch, N_("ref"),
+ N_("deepen history of shallow clone, excluding ref"),
+ 0),
+ OPT_PASSTHRU_ARGV(0, "deepen", &opt_fetch, N_("n"),
+ N_("deepen history of shallow clone"),
+ 0),
+ OPT_PASSTHRU(0, "unshallow", &opt_unshallow, NULL,
+ N_("convert to a complete repository"),
+ PARSE_OPT_NONEG | PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "update-shallow", &opt_update_shallow, NULL,
+ N_("accept refs that update .git/shallow"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "refmap", &opt_refmap, N_("refmap"),
+ N_("specify fetch refmap"),
+ PARSE_OPT_NONEG),
+ OPT_PASSTHRU_ARGV('o', "server-option", &opt_fetch,
+ N_("server-specific"),
+ N_("option to transmit"),
+ 0),
+ OPT_PASSTHRU('4', "ipv4", &opt_ipv4, NULL,
+ N_("use IPv4 addresses only"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU('6', "ipv6", &opt_ipv6, NULL,
+ N_("use IPv6 addresses only"),
+ PARSE_OPT_NOARG),
+ OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
+ N_("report that we have only objects reachable from this object"),
+ 0),
+ OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
+ N_("check for forced-updates on all updated branches")),
+ OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
+ N_("set upstream for git pull/fetch"),
+ PARSE_OPT_NOARG),
+
+ OPT_END()
+ };
+
if (!getenv("GIT_REFLOG_ACTION"))
set_reflog_message(argc, argv);
--
2.52.0
next reply other threads:[~2025-12-12 2:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-12 2:09 K Jayatheerth [this message]
2025-12-12 4:50 ` [PATCH] Make pull.c match the structural conventions Junio C Hamano
2025-12-12 7:44 ` [PATCH v2] pull: move options[] array into function scope K Jayatheerth
2025-12-12 14:08 ` [PATCH] Make pull.c match the structural conventions Kristoffer Haugsbakk
2025-12-12 14:48 ` JAYATHEERTH K
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=20251212020930.11654-1-jayatheerthkulkarni2005@gmail.com \
--to=jayatheerthkulkarni2005@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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