* [PATCH] fetch: add fetch.pruneLocalBranches config
@ 2026-05-01 21:35 Harald Nordgren via GitGitGadget
2026-05-03 22:39 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-01 21:35 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
Introduce a tri-state config option that, when --prune (or
fetch.prune / remote.<name>.prune) removes a remote-tracking
ref, also deletes local branches whose configured upstream is
that ref.
Values:
- false (default): no change in behavior.
- safe: delete only if the local tip is reachable from the
upstream tip, preserving any unpushed work.
- force: delete unconditionally; recoverable only via reflog.
The currently checked-out branch is always preserved.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
fetch: add fetch.pruneBranches config
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v1
Pull-Request: https://github.com/git/git/pull/2285
Documentation/config/fetch.adoc | 39 +++++++
Documentation/config/remote.adoc | 7 ++
Documentation/fetch-options.adoc | 9 ++
Documentation/git-fetch.adoc | 6 ++
builtin/fetch.c | 172 ++++++++++++++++++++++++++++++-
remote.c | 16 +++
remote.h | 10 ++
t/t5510-fetch.sh | 84 +++++++++++++++
8 files changed, 339 insertions(+), 4 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index cd40db0cad..5a60507a84 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -50,6 +50,45 @@
refs. See also `remote.<name>.pruneTags` and the PRUNING
section of linkgit:git-fetch[1].
+`fetch.pruneBranches`::
+ When set in addition to `fetch.prune` (or `--prune`), also
+ delete local branches whose configured upstream
+ (`branch.<name>.merge`) is one of the remote-tracking refs
+ just removed by pruning. This is useful for cleaning up topic
+ branches whose upstream counterpart has been merged and then
+ removed. The same effect can be requested per-invocation with
+ `--prune-branches[=<mode>]`, or per-remote with
+ `remote.<name>.pruneBranches`.
++
+The currently checked-out branch (in any worktree) is never
+deleted. The value is one of:
++
+--
+`false` (the default);;
+ Do not delete any local branches. Equivalent to leaving
+ the option unset.
+`safe`;;
+ Delete a local branch only if its tip is an ancestor of
+ the upstream remote-tracking ref's last-known position.
+ In other words, only delete the branch if it contains no
+ commits that the upstream did not also have at the moment
+ it was deleted. This catches the common case of a branch
+ that was pushed and then squash- or rebase-merged
+ upstream (the local branch has no extra commits beyond
+ what was pushed), but preserves any branch with unpushed
+ local work.
+`force`;;
+ Delete the local branch unconditionally, even if it
+ contains unpushed commits. Use with care: if a remote
+ branch is deleted for any reason other than that its
+ contents were merged, the corresponding local commits
+ will only be retrievable through the reflog.
+--
++
+This option has no effect unless pruning is also enabled, since
+local branches are only considered for deletion when their
+upstream remote-tracking ref is being pruned in the same fetch.
+
`fetch.all`::
If true, fetch will attempt to update all available remotes.
This behavior can be overridden by passing `--no-all` or by
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..60fd5841c6 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -87,6 +87,13 @@ remote.<name>.pruneTags::
See also `remote.<name>.prune` and the PRUNING section of
linkgit:git-fetch[1].
+remote.<name>.pruneBranches::
+ When pruning is active for this remote and this is set to `safe`
+ or `force`, also delete local branches whose upstream
+ remote-tracking ref is being pruned. Overrides
+ `fetch.pruneBranches` settings, if any. See `fetch.pruneBranches`
+ for the meaning of the values.
+
remote.<name>.promisor::
When set to true, this remote will be used to fetch promisor
objects.
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 81a9d7f9bb..0764f67cc3 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -185,6 +185,15 @@ See the PRUNING section below for more details.
+
See the PRUNING section below for more details.
+`--prune-branches[=(safe|force)]`::
+ When pruning, also delete local branches whose configured
+ upstream (`branch.<name>.merge`) is one of the remote-tracking
+ refs being pruned. With no value or `safe`, refuse to delete a
+ branch with unpushed commits; with `force`, delete it
+ regardless. The currently checked-out branch is never
+ deleted. See `fetch.pruneBranches` in linkgit:git-config[1] for
+ details.
+
endif::git-pull[]
ifndef::git-pull[]
diff --git a/Documentation/git-fetch.adoc b/Documentation/git-fetch.adoc
index db03541915..a50b9672a1 100644
--- a/Documentation/git-fetch.adoc
+++ b/Documentation/git-fetch.adoc
@@ -179,6 +179,12 @@ It's reasonable to e.g. configure `fetch.pruneTags=true` in
run, without making every invocation of `git fetch` without `--prune`
an error.
+Local branches whose upstream remote-tracking ref is being pruned can
+also be deleted automatically with `--prune-branches[=<mode>]` (or its
+config equivalents `fetch.pruneBranches` and `remote.<name>.pruneBranches`).
+See linkgit:git-config[1] for the data-loss tradeoff between the
+`safe` and `force` modes.
+
Pruning tags with `--prune-tags` also works when fetching a URL
instead of a named remote. These will all prune tags not found on
origin:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a22c319467..c6c2f00be0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -82,6 +82,21 @@ static int prune = -1; /* unspecified */
static int prune_tags = -1; /* unspecified */
#define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */
+static int prune_branches = PRUNE_BRANCHES_UNSPECIFIED;
+
+static int parse_prune_branches_opt(const struct option *opt,
+ const char *arg, int unset)
+{
+ int *v = opt->value;
+ if (unset)
+ *v = PRUNE_BRANCHES_OFF;
+ else if (arg)
+ *v = parse_prune_branches_value(opt->long_name, arg);
+ else
+ *v = PRUNE_BRANCHES_SAFE;
+ return 0;
+}
+
static int append, dry_run, force, keep, update_head_ok;
static int write_fetch_head = 1;
static int verbosity, deepen_relative, set_upstream, refetch;
@@ -105,6 +120,7 @@ struct fetch_config {
int all;
int prune;
int prune_tags;
+ enum prune_branches_mode prune_branches;
int show_forced_updates;
int recurse_submodules;
int parallel;
@@ -131,6 +147,11 @@ static int git_fetch_config(const char *k, const char *v,
return 0;
}
+ if (!strcmp(k, "fetch.prunebranches")) {
+ fetch_config->prune_branches = parse_prune_branches_value(k, v);
+ return 0;
+ }
+
if (!strcmp(k, "fetch.showforcedupdates")) {
fetch_config->show_forced_updates = git_config_bool(k, v);
return 0;
@@ -1445,7 +1466,8 @@ out:
static int prune_refs(struct display_state *display_state,
struct refspec *rs,
struct ref_transaction *transaction,
- struct ref *ref_map)
+ struct ref *ref_map,
+ struct ref **stale_refs_out)
{
int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map);
@@ -1487,7 +1509,126 @@ static int prune_refs(struct display_state *display_state,
cleanup:
string_list_clear(&refnames, 0);
strbuf_release(&err);
- free_refs(stale_refs);
+ if (!result && stale_refs_out)
+ *stale_refs_out = stale_refs;
+ else
+ free_refs(stale_refs);
+ return result;
+}
+
+struct prune_branches_cb {
+ struct string_list *pruned_refs;
+ struct string_list *to_delete;
+ struct string_list *skipped_unmerged;
+ enum prune_branches_mode mode;
+};
+
+static int collect_branches_to_prune(const struct reference *ref, void *cb_data)
+{
+ struct prune_branches_cb *cb = cb_data;
+ const char *short_name = ref->name;
+ char *full_ref = xstrfmt("refs/heads/%s", short_name);
+ const char *upstream;
+ struct string_list_item *pruned;
+ int result = 0;
+
+ if (ref->flags & REF_ISSYMREF)
+ goto out;
+ if (branch_checked_out(full_ref))
+ goto out;
+
+ upstream = branch_get_upstream(branch_get(short_name), NULL);
+ if (!upstream)
+ goto out;
+
+ pruned = string_list_lookup(cb->pruned_refs, upstream);
+ if (!pruned)
+ goto out;
+
+ if (cb->mode == PRUNE_BRANCHES_SAFE) {
+ struct commit *local = lookup_commit_reference(the_repository,
+ ref->oid);
+ struct commit *up = lookup_commit_reference(the_repository,
+ pruned->util);
+ int reachable = local && up &&
+ repo_in_merge_bases(the_repository, local, up);
+
+ if (reachable < 0) {
+ result = -1;
+ goto out;
+ }
+ if (!reachable) {
+ string_list_append(cb->skipped_unmerged, short_name);
+ goto out;
+ }
+ }
+
+ string_list_append(cb->to_delete, full_ref);
+
+out:
+ free(full_ref);
+ return result;
+}
+
+static int do_prune_branches(struct display_state *display_state,
+ struct ref *stale_refs,
+ enum prune_branches_mode mode)
+{
+ struct string_list pruned_refs = STRING_LIST_INIT_NODUP;
+ struct string_list to_delete = STRING_LIST_INIT_DUP;
+ struct string_list skipped_unmerged = STRING_LIST_INIT_DUP;
+ struct prune_branches_cb cb = {
+ .pruned_refs = &pruned_refs,
+ .to_delete = &to_delete,
+ .skipped_unmerged = &skipped_unmerged,
+ .mode = mode,
+ };
+ struct ref *ref;
+ struct string_list_item *item;
+ int result = 0;
+
+ if (!stale_refs)
+ return 0;
+
+ for (ref = stale_refs; ref; ref = ref->next)
+ string_list_append(&pruned_refs, ref->name)->util = &ref->new_oid;
+ string_list_sort(&pruned_refs);
+
+ if (refs_for_each_branch_ref(get_main_ref_store(the_repository),
+ collect_branches_to_prune, &cb)) {
+ result = -1;
+ goto cleanup;
+ }
+
+ if (!dry_run && to_delete.nr)
+ result = refs_delete_refs(get_main_ref_store(the_repository),
+ "fetch: prune branches",
+ &to_delete, REF_NO_DEREF);
+
+ if (verbosity >= 0) {
+ const struct object_id *zero = null_oid(the_repository->hash_algo);
+ for_each_string_list_item(item, &to_delete) {
+ const char *short_name;
+ if (skip_prefix(item->string, "refs/heads/", &short_name))
+ display_ref_update(display_state, '-',
+ _("[deleted local]"), NULL,
+ _("(none)"), short_name,
+ zero, zero,
+ transport_summary_width(NULL));
+ }
+ }
+ for_each_string_list_item(item, &skipped_unmerged)
+ warning(_("not deleting local branch '%s' that is not "
+ "fully merged into its upstream;\n"
+ " set fetch.pruneBranches=force to "
+ "delete anyway, or delete manually with "
+ "'git branch -D %s'"),
+ item->string, item->string);
+
+cleanup:
+ string_list_clear(&pruned_refs, 0);
+ string_list_clear(&to_delete, 0);
+ string_list_clear(&skipped_unmerged, 0);
return result;
}
@@ -1945,19 +2086,28 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
if (prune) {
+ struct ref *stale_refs = NULL;
+ struct ref **stale_refs_out = prune_branches != PRUNE_BRANCHES_OFF
+ ? &stale_refs : NULL;
/*
* We only prune based on refspecs specified
* explicitly (via command line or configuration); we
* don't care whether --tags was specified.
*/
if (rs->nr) {
- retcode = prune_refs(&display_state, rs, transaction, ref_map);
+ retcode = prune_refs(&display_state, rs, transaction,
+ ref_map, stale_refs_out);
} else {
retcode = prune_refs(&display_state, &transport->remote->fetch,
- transaction, ref_map);
+ transaction, ref_map, stale_refs_out);
}
if (retcode != 0)
retcode = 1;
+ else if (stale_refs &&
+ do_prune_branches(&display_state, stale_refs,
+ prune_branches))
+ retcode = 1;
+ free_refs(stale_refs);
}
/*
@@ -2419,6 +2569,16 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
prune_tags = PRUNE_TAGS_BY_DEFAULT;
}
+ if (prune_branches == PRUNE_BRANCHES_UNSPECIFIED) {
+ /* no command line request */
+ if (remote->prune_branches >= 0)
+ prune_branches = remote->prune_branches;
+ else if (config->prune_branches >= 0)
+ prune_branches = config->prune_branches;
+ else
+ prune_branches = PRUNE_BRANCHES_OFF;
+ }
+
maybe_prune_tags = prune_tags_ok && prune_tags;
if (maybe_prune_tags && remote_via_config)
refspec_append(&remote->fetch, TAG_REFSPEC);
@@ -2469,6 +2629,7 @@ int cmd_fetch(int argc,
.display_format = DISPLAY_FORMAT_FULL,
.prune = -1,
.prune_tags = -1,
+ .prune_branches = PRUNE_BRANCHES_UNSPECIFIED,
.show_forced_updates = 1,
.recurse_submodules = RECURSE_SUBMODULES_DEFAULT,
.parallel = 1,
@@ -2520,6 +2681,9 @@ int cmd_fetch(int argc,
N_("prune remote-tracking branches no longer on remote")),
OPT_BOOL('P', "prune-tags", &prune_tags,
N_("prune local tags no longer on remote and clobber changed tags")),
+ OPT_CALLBACK_F(0, "prune-branches", &prune_branches, N_("mode"),
+ N_("delete local branches whose upstream was pruned ('safe' or 'force')"),
+ PARSE_OPT_OPTARG, parse_prune_branches_opt),
OPT_CALLBACK_F(0, "recurse-submodules", &recurse_submodules_cli, N_("on-demand"),
N_("control recursive fetching of submodules"),
PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),
diff --git a/remote.c b/remote.c
index a664cd166a..1e2b4803e7 100644
--- a/remote.c
+++ b/remote.c
@@ -148,6 +148,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
CALLOC_ARRAY(ret, 1);
ret->prune = -1; /* unspecified */
ret->prune_tags = -1; /* unspecified */
+ ret->prune_branches = -1; /* unspecified */
ret->name = xstrndup(name, len);
refspec_init_push(&ret->push);
refspec_init_fetch(&ret->fetch);
@@ -423,6 +424,19 @@ out:
}
#endif /* WITH_BREAKING_CHANGES */
+int parse_prune_branches_value(const char *k, const char *v)
+{
+ if (v) {
+ if (!strcasecmp(v, "safe"))
+ return PRUNE_BRANCHES_SAFE;
+ if (!strcasecmp(v, "force"))
+ return PRUNE_BRANCHES_FORCE;
+ }
+ if (git_parse_maybe_bool(v) == 0)
+ return PRUNE_BRANCHES_OFF;
+ die(_("invalid value for '%s': '%s'"), k, v);
+}
+
static int handle_config(const char *key, const char *value,
const struct config_context *ctx, void *cb)
{
@@ -507,6 +521,8 @@ static int handle_config(const char *key, const char *value,
remote->prune = git_config_bool(key, value);
else if (!strcmp(subkey, "prunetags"))
remote->prune_tags = git_config_bool(key, value);
+ else if (!strcmp(subkey, "prunebranches"))
+ remote->prune_branches = parse_prune_branches_value(key, value);
else if (!strcmp(subkey, "url")) {
if (!value)
return config_error_nonbool(key);
diff --git a/remote.h b/remote.h
index fc052945ee..5b750c8229 100644
--- a/remote.h
+++ b/remote.h
@@ -28,6 +28,15 @@ enum {
#endif /* WITH_BREAKING_CHANGES */
};
+enum prune_branches_mode {
+ PRUNE_BRANCHES_UNSPECIFIED = -1,
+ PRUNE_BRANCHES_OFF = 0,
+ PRUNE_BRANCHES_SAFE,
+ PRUNE_BRANCHES_FORCE,
+};
+
+int parse_prune_branches_value(const char *k, const char *v);
+
struct rewrite {
const char *base;
size_t baselen;
@@ -102,6 +111,7 @@ struct remote {
int mirror;
int prune;
int prune_tags;
+ int prune_branches;
/**
* The configured helper programs to run on the remote side, for
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 6fe21e2b3a..5a2ff40132 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -386,6 +386,90 @@ test_expect_success REFFILES 'fetch --prune fails to delete branches' '
)
'
+test_expect_success 'fetch.pruneBranches: setup parent' '
+ git init -b main prune-branches-parent &&
+ test_commit -C prune-branches-parent base
+'
+
+test_expect_success 'fetch.pruneBranches=safe deletes merged local branch' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-safe &&
+ git -C prune-branches-safe checkout -b doomed --track origin/doomed &&
+ git -C prune-branches-safe checkout -b stay &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-safe -c fetch.pruneBranches=safe fetch --prune origin &&
+ test_must_fail git -C prune-branches-safe rev-parse refs/remotes/origin/doomed &&
+ test_must_fail git -C prune-branches-safe rev-parse refs/heads/doomed
+'
+
+test_expect_success 'fetch.pruneBranches=safe keeps unmerged local branch' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-safe-unmerged &&
+ git -C prune-branches-safe-unmerged checkout -b doomed --track origin/doomed &&
+ test_commit -C prune-branches-safe-unmerged local-only &&
+ git -C prune-branches-safe-unmerged checkout -b stay &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-safe-unmerged -c fetch.pruneBranches=safe fetch --prune origin 2>err &&
+ test_must_fail git -C prune-branches-safe-unmerged rev-parse refs/remotes/origin/doomed &&
+ git -C prune-branches-safe-unmerged rev-parse refs/heads/doomed &&
+ test_grep "not fully merged" err
+'
+
+test_expect_success 'fetch.pruneBranches=force deletes unmerged local branch' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-force &&
+ git -C prune-branches-force checkout -b doomed --track origin/doomed &&
+ test_commit -C prune-branches-force local-only-force &&
+ git -C prune-branches-force checkout -b stay &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-force -c fetch.pruneBranches=force fetch --prune origin &&
+ test_must_fail git -C prune-branches-force rev-parse refs/remotes/origin/doomed &&
+ test_must_fail git -C prune-branches-force rev-parse refs/heads/doomed
+'
+
+test_expect_success 'fetch.pruneBranches=force never deletes checked-out branch' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-checked-out &&
+ git -C prune-branches-checked-out checkout -b doomed --track origin/doomed &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-checked-out -c fetch.pruneBranches=force fetch --prune origin &&
+ test_must_fail git -C prune-branches-checked-out rev-parse refs/remotes/origin/doomed &&
+ git -C prune-branches-checked-out rev-parse refs/heads/doomed
+'
+
+test_expect_success '--prune-branches deletes merged local branch' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-cli &&
+ git -C prune-branches-cli checkout -b doomed --track origin/doomed &&
+ git -C prune-branches-cli checkout -b stay &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-cli fetch --prune --prune-branches origin &&
+ test_must_fail git -C prune-branches-cli rev-parse refs/heads/doomed
+'
+
+test_expect_success '--no-prune-branches overrides fetch.pruneBranches' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-no-cli &&
+ git -C prune-branches-no-cli checkout -b doomed --track origin/doomed &&
+ git -C prune-branches-no-cli checkout -b stay &&
+ git -C prune-branches-no-cli config fetch.pruneBranches force &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-no-cli fetch --prune --no-prune-branches origin &&
+ git -C prune-branches-no-cli rev-parse refs/heads/doomed
+'
+
+test_expect_success 'remote.<name>.pruneBranches overrides fetch.pruneBranches' '
+ git -C prune-branches-parent branch doomed base &&
+ git clone prune-branches-parent prune-branches-per-remote &&
+ git -C prune-branches-per-remote checkout -b doomed --track origin/doomed &&
+ git -C prune-branches-per-remote checkout -b stay &&
+ git -C prune-branches-per-remote config fetch.pruneBranches force &&
+ git -C prune-branches-per-remote config remote.origin.pruneBranches false &&
+ git -C prune-branches-parent branch -D doomed &&
+ git -C prune-branches-per-remote fetch --prune origin &&
+ git -C prune-branches-per-remote rev-parse refs/heads/doomed
+'
+
test_expect_success 'fetch --atomic works with a single branch' '
test_when_finished "rm -rf atomic" &&
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-01 21:35 Harald Nordgren via GitGitGadget
@ 2026-05-03 22:39 ` Junio C Hamano
2026-05-05 7:14 ` Johannes Sixt
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-05-03 22:39 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> Introduce a tri-state config option that, when --prune (or
> fetch.prune / remote.<name>.prune) removes a remote-tracking
> ref, also deletes local branches whose configured upstream is
> that ref.
>
> Values:
> - false (default): no change in behavior.
> - safe: delete only if the local tip is reachable from the
> upstream tip, preserving any unpushed work.
> - force: delete unconditionally; recoverable only via reflog.
>
> The currently checked-out branch is always preserved.
I do like the feature that allows you to identify which local
branches are already merged and prune them. It will help users keep
their local branch namespace clean.
I however do not like to see the feature tied to "fetch". By this,
I do not mean I do not want an option to trigger the feature when
"git fetch" is run. What I mean is that users should have an option
to prune merged branches without having to fetch first. And you can
then optionally trigger that machinery from "git fetch".
Of course they aleady can do something silly like
$ git branch -d $(git branch --list | sed -e 's/^..//')
and remove all the merged branches, but compared to what is
presented here, one thing missing is that you allow pruning the
local branches that are merged only to remote-tracking branches from
a single remote.
To break the feature down to make it easier to use by our users with
various needs and workflows, we would benefit from having a
collection of smaller features that can be composed, like these:
* "git branch --forked <remote>" lists local branches that build on
something taken from <remote>s. The option can be given multiple
times to make a union of the results from individual "--forked
<remote>".
- <remote> may be a name of a remote, e.g., "origin" to mean all
the remote-tracking branches "refs/remotes/origin/*",
- <remote> may be "origin/master" to name a specific
remote-tracking branch.
- There may be other handy things to cover with <remote>, like
"--all" that may act as if you listed all the available
<remote> on the command line.
* "git branch --prune-merged <remote>..." is a short-hand for "git
branch -d $(git branch --forked <remote>...".
* "git fetch/pull --prune-merged <remote>" can trigger "git branch
--prune-merged <remote>" after "git fetch" successfully updates
the remote-tracking branches, which should be equivalent to what
you have here..
Some local branches that fork from remote and have their initial
round already merged may not want to be pruned, however. You may
have multi-stage development plans for that topic, and you know
already the second phase would want to build on top of the initial
round, not a random version of the mainline with many topics from
other folks merged in. So you'd rather want to keep the topic
branch around after your initial round has been merged to the
upstream before you start the second phase. This is especially true
if your topic is designed to apply to an existing release (in other
words, a bugfix) and you want to keep the second and subsequent
rounds of the topic to be applicable to the same target version
without contaminating the topic with irrelevant features from others
that happened to have been developed and merged upstream around the
same time.
And we'd need to cater to their needs. By this, I do not mean "they
do not have to use --prune-merged", but by giving them a way to say
"this branch should not be auto-pruned with --prune-merged".
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-03 22:39 ` Junio C Hamano
@ 2026-05-05 7:14 ` Johannes Sixt
0 siblings, 0 replies; 36+ messages in thread
From: Johannes Sixt @ 2026-05-05 7:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Harald Nordgren, Harald Nordgren via GitGitGadget
Am 04.05.26 um 00:39 schrieb Junio C Hamano:
> To break the feature down to make it easier to use by our users with
> various needs and workflows, we would benefit from having a
> collection of smaller features that can be composed, like these:
>
> * "git branch --forked <remote>" lists local branches that build on
> something taken from <remote>s. The option can be given multiple
> times to make a union of the results from individual "--forked
> <remote>".
Clearly, this version of --forked does something very different from the
option `--merged some_branch` that we already have.
>
> - <remote> may be a name of a remote, e.g., "origin" to mean all
> the remote-tracking branches "refs/remotes/origin/*",
>
> - <remote> may be "origin/master" to name a specific
> remote-tracking branch.
>
> - There may be other handy things to cover with <remote>, like
> "--all" that may act as if you listed all the available
> <remote> on the command line.
> > * "git branch --prune-merged <remote>..." is a short-hand for "git
> branch -d $(git branch --forked <remote>...".
I don't understand this. The option includes the word "merged". Then I
interpret the command to prune only branches that have already been
merged into something (BTW, merged into what?), but as described, the
command removes all local branches that have been forked from some
(remote) branch.
>
> * "git fetch/pull --prune-merged <remote>" can trigger "git branch
> --prune-merged <remote>" after "git fetch" successfully updates
> the remote-tracking branches, which should be equivalent to what
> you have here..
I think that the intended behavior is to call the equivalent of `git
branch --merged X | xargs git branch -d` for a suitable set of 'X' to be
determined by `git fetch`.
-- Hannes
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-05 20:48 [PATCH v4 4/6] fetch: add --prune-merged Johannes Sixt
@ 2026-05-05 22:07 ` Harald Nordgren
2026-05-11 2:59 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: Harald Nordgren @ 2026-05-05 22:07 UTC (permalink / raw)
To: j6t; +Cc: git, gitgitgadget, haraldnordgren, kristofferhaugsbakk
> I have some sympathy for the desire to clean up unnecessary local
> branches, but I don't like the concept that `git fetch` modifies local
> branches, not even as an opt-in. Deleting local branches should be `git
> branch`'s task exclusively (at the porcelain level).
Yeah, maybe that's a good point.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-05 22:07 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
@ 2026-05-11 2:59 ` Junio C Hamano
2026-05-11 6:56 ` Harald Nordgren
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-05-11 2:59 UTC (permalink / raw)
To: Harald Nordgren; +Cc: j6t, git, gitgitgadget, kristofferhaugsbakk
Harald Nordgren <haraldnordgren@gmail•com> writes:
>> I have some sympathy for the desire to clean up unnecessary local
>> branches, but I don't like the concept that `git fetch` modifies local
>> branches, not even as an opt-in. Deleting local branches should be `git
>> branch`'s task exclusively (at the porcelain level).
>
> Yeah, maybe that's a good point.
I think the latest iteration was sent after the above exchange, yet
it seems to have changes to builtin/fetch.c to cause `git fetch` to
modify local branches still. Will we have another update that is
hopefully final to excise that part, or are we OK to allow `fetch`
to modify the local state as an opt-in now?
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-11 2:59 ` Junio C Hamano
@ 2026-05-11 6:56 ` Harald Nordgren
0 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-11 6:56 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
>>> I have some sympathy for the desire to clean up unnecessary local
>>> branches, but I don't like the concept that `git fetch` modifies local
>>> branches, not even as an opt-in. Deleting local branches should be `git
>>> branch`'s task exclusively (at the porcelain level).
>>
>> Yeah, maybe that's a good point.
>
> I think the latest iteration was sent after the above exchange, yet
> it seems to have changes to builtin/fetch.c to cause `git fetch` to
> modify local branches still. Will we have another update that is
> hopefully final to excise that part, or are we OK to allow `fetch`
> to modify the local state as an opt-in now?
Done! (I didn't know if we wanted to do this yet, or we still just
discussion it, but now I deleted it.)
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-11 8:18 [PATCH v5 2/5] branch: let delete_branches warn instead of error on bulk refusal Junio C Hamano
@ 2026-05-11 8:44 ` Harald Nordgren
0 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-11 8:44 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
> Existing call sites are about "branch -d <other>" that allows the
> other branch to be deleted if it is part of HEAD or if it is part of
> its tracking branch, but should "branch --prune-merged" pay
> attention to what branch happens to be checked out the same way (not
> a rherotical question to hint that I do not think it should---I do
> not have a strong opinion on this either way)?
This is a very good question! My opion is that it should work the same way
regardless of which branch you are on, it should always compare against the
remote's default branch.
I this explains some weirdness I saw today when running it from non-main
and prune didn't get triggered.
I will look into making that change.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-11 23:20 [PATCH v6 0/5] branch: prune-merged Junio C Hamano
@ 2026-05-12 7:35 ` Harald Nordgren
0 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-12 7:35 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
> I may be misreading the above and misunderstood you, but if you mean
> that the feature now checks with remote/origin/master when I have a
> local branch that were forked from remote/origin/todo and set to
> merge new changes from there, I do not think it is a good change.
I think you are right. My latest code assumes that everyone works toward
the default branch, which is what I do 99% of the time, but yeah, it should
be more agnostic from different workflow.
I'll take another look.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-12 13:53 [PATCH v7 3/5] branch: add --prune-merged <remote> Junio C Hamano
@ 2026-05-12 17:00 ` Harald Nordgren
0 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-12 17:00 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
> But the above description uses quite different condition. You want
> to notice that _they_ removed 'next' (for that, the second-to-last
> fetch may need to be run with --prune) and then remove our local
> hn/topic, but to me, that sounds nonsense for two reasons.
I wanted to be able to be aggresice with deleting, but maybe this went a
bit overboard. Would still be nice to have a nuclear option.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] config: suggest the correct form when key contains "="
@ 2026-05-13 13:58 Harald Nordgren via GitGitGadget
2026-05-14 21:26 ` Junio C Hamano
2026-05-16 12:52 ` [PATCH v2] config: suggest the correct form when key contains "=" in set context Harald Nordgren via GitGitGadget
0 siblings, 2 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-13 13:58 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
When a user types "git config foo.bar=baz", git_config_parse_key()
rejects the key with "error: invalid key: foo.bar=baz" but gives no
indication of what the user should have written. The mistake is a
common one for users who reach for INI-file syntax or for the
"--flag=value" convention used by other command-line tools.
Since "=" is never a valid character in a config key, treat its
presence as a strong signal of this specific mistake and follow the
error with a one-line suggestion in the "(did you mean ...)" style
used elsewhere in git, e.g.:
$ git config pull.rebase=false
error: invalid key: pull.rebase=false
(did you mean "git config set pull.rebase false"?)
The hint is emitted only when the offending character is "="; other
invalid characters (newlines, "@", etc.) keep their existing error
unchanged.
Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
---
config: suggest the correct form when key contains "="
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v1
Pull-Request: https://github.com/git/git/pull/2302
config.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/config.c b/config.c
index a1b92fe083..6e658d71d1 100644
--- a/config.c
+++ b/config.c
@@ -580,6 +580,10 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
error(_("invalid key: %s"), key);
+ if (c == '=')
+ fprintf_ln(stderr,
+ _(" (did you mean \"git config set %.*s %s\"?)"),
+ (int)i, key, key + i + 1);
goto out_free_ret_1;
}
c = tolower(c);
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-13 13:46 [PATCH v8 0/5] branch: prune-merged Junio C Hamano
@ 2026-05-13 18:57 ` Harald Nordgren
0 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-13 18:57 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren, j6t, kristofferhaugsbakk
> I am not sure if this is a good idea at all. The option is called
> prune-MERGED and with or without --force, mergedness should be what
> determines if a branch is deleted.
Well, when I started writing the feature it was "prune local branches",
and it evolved from there to prune merged.
But you're probably right. I did wipe up some branches with real work on my
side using this (I restored them), so it seems to be more of a foot-gun
than I first imagined.
Seems reasonable to remove the '--force' functionality.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] config: suggest the correct form when key contains "="
2026-05-13 13:58 [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
@ 2026-05-14 21:26 ` Junio C Hamano
2026-05-14 22:16 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-16 12:51 ` [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren
2026-05-16 12:52 ` [PATCH v2] config: suggest the correct form when key contains "=" in set context Harald Nordgren via GitGitGadget
1 sibling, 2 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-05-14 21:26 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> From: Harald Nordgren <haraldnordgren@gmail•com>
>
> When a user types "git config foo.bar=baz", git_config_parse_key()
> rejects the key with "error: invalid key: foo.bar=baz" but gives no
> indication of what the user should have written. The mistake is a
> common one for users who reach for INI-file syntax or for the
> "--flag=value" convention used by other command-line tools.
>
> Since "=" is never a valid character in a config key, treat its
> presence as a strong signal of this specific mistake and follow the
> error with a one-line suggestion in the "(did you mean ...)" style
> used elsewhere in git, e.g.:
>
> $ git config pull.rebase=false
> error: invalid key: pull.rebase=false
> (did you mean "git config set pull.rebase false"?)
If the command line were
git config get foo.bar=baz
git config set foo.bar=baz nitfol
we shouldn't give an extra "did you mean?" at all.
The only cases you may want to do the "did you mean?" I think are
git config foo.bar=baz
git config set foo.bar=baz
And I think git_config_parse_key() is at a way too low level to tell
in what context we are seeing this faulty key to guess end-user's
intention to limit our "did you mean?"
I also wonder if, given that "=" in anywhere other than three-level
names, is invalid, we should just start accept
git config foo.bar=baz
git config set foo.bar=baz
and interpret them as
git config set foo.bar baz
We of course need to be careful about non-invalid keys, i.e.
git config foo.bar=baz.boo
is a request to read the value of that named variable, i.e.
[foo "bar=baz"]
boo = its value
so either you start offering unsolicited "did you mean?" or accepting
tokens with '=' in them as new style "set", you need to be extra
careful not to trigger a false positive.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-14 21:26 ` Junio C Hamano
@ 2026-05-14 22:16 ` Harald Nordgren
2026-05-15 1:28 ` Junio C Hamano
2026-05-16 12:51 ` [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren
1 sibling, 1 reply; 36+ messages in thread
From: Harald Nordgren @ 2026-05-14 22:16 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren
> I also wonder if, given that "=" in anywhere other than three-level
> names, is invalid, we should just start accept
>
> git config foo.bar=baz
> git config set foo.bar=baz
>
> and interpret them as
>
> git config set foo.bar baz
That sounds good too! Probably even better.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-14 22:16 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
@ 2026-05-15 1:28 ` Junio C Hamano
2026-05-15 7:56 ` Email issues Harald Nordgren
2026-05-15 9:39 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
0 siblings, 2 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-05-15 1:28 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, gitgitgadget
Harald Nordgren <haraldnordgren@gmail•com> writes:
>> I also wonder if, given that "=" in anywhere other than three-level
>> names, is invalid, we should just start accept
>>
>> git config foo.bar=baz
>> git config set foo.bar=baz
>>
>> and interpret them as
>>
>> git config set foo.bar baz
>
> That sounds good too! Probably even better.
>
>
> Harald
Why do I get the above, which apparently is a response to my review
for
[PATCH] config: suggest the correct form when key contains "="
under this thread? Am I dealing with some sort of mechanical slop?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Email issues
2026-05-15 1:28 ` Junio C Hamano
@ 2026-05-15 7:56 ` Harald Nordgren
2026-05-15 12:02 ` Kristoffer Haugsbakk
2026-05-15 9:39 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
1 sibling, 1 reply; 36+ messages in thread
From: Harald Nordgren @ 2026-05-15 7:56 UTC (permalink / raw)
To: gitster; +Cc: git, gitgitgadget, haraldnordgren
> Why do I get the above, which apparently is a response to my review
> for
>
> [PATCH] config: suggest the correct form when key contains "="
>
> under this thread? Am I dealing with some sort of mechanical slop?
I think the problem here is my email sending process is not good. I edit
all the emails in Sublime text, where I keep the same file for all
different threads.
I have the subject line as the first line of the file and like you notice I
forget to change it sometimes.
I keep each of the topics bookmarked like this,
https://lore.kernel.org/git/xmqqecjdea13.fsf@gitster.g/, and then utilize
that like to send the email
```
git send-email \
--in-reply-to=xmqqecjdea13.fsf@gitster•g \
--to=gitster@pobox•com \
--cc=git@vger•kernel.org \
--cc=gitgitgadget@gmail•com \
--cc=haraldnordgren@gmail•com \
/path/to/YOUR_REPLY
```
I tried playing with neomutt and and email client replacement, but that
adds the complexity of downloading a new mbox file for each reply, it
didn't seem easier, but maybe it is.
How do you handle emails?
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] fetch: add fetch.pruneLocalBranches config
2026-05-15 1:28 ` Junio C Hamano
2026-05-15 7:56 ` Email issues Harald Nordgren
@ 2026-05-15 9:39 ` Harald Nordgren
1 sibling, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-15 9:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, gitgitgadget
> Why do I get the above, which apparently is a response to my review
> for
>
> [PATCH] config: suggest the correct form when key contains "="
>
> under this thread? Am I dealing with some sort of mechanical slop?
(Testing plain text email sending via Gmail for a less error-prone
workflow, does it still add the CC's correctly?)
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: Email issues
2026-05-15 7:56 ` Email issues Harald Nordgren
@ 2026-05-15 12:02 ` Kristoffer Haugsbakk
0 siblings, 0 replies; 36+ messages in thread
From: Kristoffer Haugsbakk @ 2026-05-15 12:02 UTC (permalink / raw)
To: Harald Nordgren, Junio C Hamano; +Cc: git, Koji Nakamaru
On Fri, May 15, 2026, at 09:56, Harald Nordgren wrote:
>> Why do I get the above, which apparently is a response to my review
>> for
>>
>> [PATCH] config: suggest the correct form when key contains "="
>>
>> under this thread? Am I dealing with some sort of mechanical slop?
>
> I think the problem here is my email sending process is not good. I edit
> all the emails in Sublime text, where I keep the same file for all
> different threads.
>
> I have the subject line as the first line of the file and like you notice I
> forget to change it sometimes.
>
> I keep each of the topics bookmarked like this,
> https://lore.kernel.org/git/xmqqecjdea13.fsf@gitster.g/, and then utilize
> that like to send the email
>
> ```
> git send-email \
> --in-reply-to=xmqqecjdea13.fsf@gitster•g \
> --to=gitster@pobox•com \
> --cc=git@vger•kernel.org \
> --cc=gitgitgadget@gmail•com \
> --cc=haraldnordgren@gmail•com \
> /path/to/YOUR_REPLY
> ```
>
> I tried playing with neomutt and and email client replacement, but that
> adds the complexity of downloading a new mbox file for each reply, it
> didn't seem easier, but maybe it is.
>
> How do you handle emails?
I use the Fastmail webmail client for
regular non-patch emails. The only
things it messes up so far is long lines
in replies to patches.
I edit the emails in a text editor. And sometimes
I have left multiple drafts before sending them
and switched them around. Only to see my mistake on the Lore archive later. :)
But by and large it works just fine. I haven't had
the need for a more ergonomic setup.
--
Sent from mobile
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH] config: suggest the correct form when key contains "="
2026-05-14 21:26 ` Junio C Hamano
2026-05-14 22:16 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
@ 2026-05-16 12:51 ` Harald Nordgren
1 sibling, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-16 12:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
> And I think git_config_parse_key() is at a way too low level to tell
> in what context we are seeing this faulty key to guess end-user's
> intention to limit our "did you mean?"
>
> I also wonder if, given that "=" in anywhere other than three-level
> names, is invalid, we should just start accept
>
> git config foo.bar=baz
> git config set foo.bar=baz
>
> and interpret them as
>
> git config set foo.bar baz
I tried implementing a version to be more liberal in what to accept, but
the implementation became very complex.
Moving in the other direction: show the warning, but try to make it more
correct.
(Also switching over to replying to emails with Gmail with 'plain text
mode'), hopefully there will be less miss-sends that end up on the wrong
topic from now on.)
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2] config: suggest the correct form when key contains "=" in set context
2026-05-13 13:58 [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-05-14 21:26 ` Junio C Hamano
@ 2026-05-16 12:52 ` Harald Nordgren via GitGitGadget
2026-05-25 8:33 ` [PATCH v3] " Harald Nordgren via GitGitGadget
1 sibling, 1 reply; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-16 12:52 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
A user who types "git config pull.rebase=false" gets only "error:
invalid key: pull.rebase=false" with no clue what went wrong.
Emit a "did you mean ..." hint suggesting the split form. Restrict it
to plausible-set contexts ("git config set", bare "git config <key>",
and their 2-arg forms); explicit "get"/"unset" keep the existing error.
"=" is legal inside a subsection, so only fire when "=" lands after
the last ".". When the user supplied a separate value, use it in the
suggestion instead of the suffix after "=":
$ git config set pull.rebase=false true
error: invalid key: pull.rebase=false
hint: did you mean "git config set pull.rebase true"?
Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
---
config: suggest the correct form when key contains "="
* Hint moved from git_config_parse_key() to a new
advise_setting_with_equals() in builtin/config.c; wired only into set
and bare paths.
* Only fires when = is after the last .; 2-arg forms use the user's
value.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v2
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v1:
1: 56eb3ce6fd < -: ---------- config: suggest the correct form when key contains "="
-: ---------- > 1: 40d9eb3e5c config: suggest the correct form when key contains "=" in set context
builtin/config.c | 30 ++++++++++++++++++++++++++++++
t/t1300-config.sh | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..f14a30e720 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,22 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static void advise_setting_with_equals(const char *key, const char *value)
+{
+ const char *last_dot = strrchr(key, '.');
+ const char *eq;
+
+ if (!last_dot)
+ return;
+ eq = strchr(last_dot + 1, '=');
+ if (!eq)
+ return;
+ if (!value)
+ value = eq + 1;
+ advise(_("did you mean \"git config set %.*s %s\"?"),
+ (int)(eq - key), key, value);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1150,11 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1 && strchr(argv[0], '=')) {
+ error(_("wrong number of arguments, should be 2"));
+ advise_setting_with_equals(argv[0], NULL);
+ exit(129);
+ }
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1160,6 +1182,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
error(_("cannot overwrite multiple values with a single value\n"
" Use --value=<pattern>, --append or --all to change %s."), argv[0]);
}
+ if (ret == CONFIG_INVALID_KEY)
+ advise_setting_with_equals(argv[0], argv[1]);
location_options_release(&location_opts);
free(comment);
@@ -1371,6 +1395,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,6 +1410,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
+ actions_implicit = (actions == 0);
if (actions == 0)
switch (argc) {
case 1: actions = ACTION_GET; break;
@@ -1485,6 +1511,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
if (ret == CONFIG_NOTHING_SET)
error(_("cannot overwrite multiple values with a single value\n"
" Use a regexp, --add or --replace-all to change %s."), argv[0]);
+ else if (ret == CONFIG_INVALID_KEY)
+ advise_setting_with_equals(argv[0], argv[1]);
}
else if (actions == ACTION_SET_ALL) {
check_write(&location_opts.source);
@@ -1515,6 +1543,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
check_argc(argc, 1, 2);
ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
0, flags);
+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
+ advise_setting_with_equals(argv[0], NULL);
}
else if (actions == ACTION_GET_ALL) {
check_argc(argc, 1, 2);
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 128971ee12..f46c081413 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -462,6 +462,53 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
+ test_must_fail git config pull.rebase=false true 2>err &&
+ test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set with single arg hints' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "wrong number of arguments" err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
+ test_must_fail git config --get pull.rebase=false 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
+ test_must_fail git config get pull.rebase=false 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
+ test_must_fail git config unset pull.rebase=false 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid, no hint' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
+ test_grep ! "did you mean" err &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3] config: suggest the correct form when key contains "=" in set context
2026-05-16 12:52 ` [PATCH v2] config: suggest the correct form when key contains "=" in set context Harald Nordgren via GitGitGadget
@ 2026-05-25 8:33 ` Harald Nordgren via GitGitGadget
2026-05-25 9:15 ` Junio C Hamano
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
0 siblings, 2 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-25 8:33 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
A user who types "git config pull.rebase=false" gets only "error:
invalid key: pull.rebase=false" with no clue what went wrong.
Emit a "did you mean ..." hint suggesting the split form. Restrict it
to plausible-set contexts ("git config set", bare "git config <key>",
and their 2-arg forms); explicit "get"/"unset" keep the existing error.
"=" is legal inside a subsection, so only fire when "=" lands after
the last ".". When the user supplied a separate value, use it in the
suggestion instead of the suffix after "=":
$ git config set pull.rebase=false true
error: invalid key: pull.rebase=false
hint: did you mean "git config set pull.rebase true"?
Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
config: suggest the correct form when key contains "="
* Skip the hint when the inferred value contains whitespace, so git
config set pull.rebase=false "hello world" no longer suggests a
malformed command.
* Replace the inline actions == 0 check with a named actions_implicit
flag, simplfied the code.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v3
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v2:
1: 40d9eb3e5c ! 1: 6b9d66361d config: suggest the correct form when key contains "=" in set context
@@ Commit message
hint: did you mean "git config set pull.rebase true"?
Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
+ Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
## builtin/config.c ##
@@
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
+ return;
+ if (!value)
+ value = eq + 1;
++ if (!*value || strpbrk(value, " \t\n"))
++ return;
+ advise(_("did you mean \"git config set %.*s %s\"?"),
+ (int)(eq - key), key, value);
+}
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
exit(129);
}
+- if (actions == 0)
+ actions_implicit = (actions == 0);
- if (actions == 0)
++ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
+ case 2: actions = ACTION_SET; break;
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
if (ret == CONFIG_NOTHING_SET)
error(_("cannot overwrite multiple values with a single value\n"
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+ test_grep ! "did you mean" err
+'
+
++test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
++ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
++ test_grep "invalid key: pull\\.rebase=false" err &&
++ test_grep ! "did you mean" err
++'
++
+test_expect_success '"=" inside subsection is valid, no hint' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
builtin/config.c | 34 +++++++++++++++++++++++++++++-
t/t1300-config.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..8c7ab36fcb 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,24 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static void advise_setting_with_equals(const char *key, const char *value)
+{
+ const char *last_dot = strrchr(key, '.');
+ const char *eq;
+
+ if (!last_dot)
+ return;
+ eq = strchr(last_dot + 1, '=');
+ if (!eq)
+ return;
+ if (!value)
+ value = eq + 1;
+ if (!*value || strpbrk(value, " \t\n"))
+ return;
+ advise(_("did you mean \"git config set %.*s %s\"?"),
+ (int)(eq - key), key, value);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1152,11 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1 && strchr(argv[0], '=')) {
+ error(_("wrong number of arguments, should be 2"));
+ advise_setting_with_equals(argv[0], NULL);
+ exit(129);
+ }
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1160,6 +1184,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
error(_("cannot overwrite multiple values with a single value\n"
" Use --value=<pattern>, --append or --all to change %s."), argv[0]);
}
+ if (ret == CONFIG_INVALID_KEY)
+ advise_setting_with_equals(argv[0], argv[1]);
location_options_release(&location_opts);
free(comment);
@@ -1371,6 +1397,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1412,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1485,6 +1513,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
if (ret == CONFIG_NOTHING_SET)
error(_("cannot overwrite multiple values with a single value\n"
" Use a regexp, --add or --replace-all to change %s."), argv[0]);
+ else if (ret == CONFIG_INVALID_KEY)
+ advise_setting_with_equals(argv[0], argv[1]);
}
else if (actions == ACTION_SET_ALL) {
check_write(&location_opts.source);
@@ -1515,6 +1545,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
check_argc(argc, 1, 2);
ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
0, flags);
+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
+ advise_setting_with_equals(argv[0], NULL);
}
else if (actions == ACTION_GET_ALL) {
check_argc(argc, 1, 2);
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..4e12b78536 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,59 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
+ test_must_fail git config pull.rebase=false true 2>err &&
+ test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set with single arg hints' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "wrong number of arguments" err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
+ test_must_fail git config --get pull.rebase=false 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
+ test_must_fail git config get pull.rebase=false 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
+ test_must_fail git config unset pull.rebase=false 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
+ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid, no hint' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
+ test_grep ! "did you mean" err &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
base-commit: 6a4418c36d6bad69a599044b3cf49dcbd049cb45
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3] config: suggest the correct form when key contains "=" in set context
2026-05-25 8:33 ` [PATCH v3] " Harald Nordgren via GitGitGadget
@ 2026-05-25 9:15 ` Junio C Hamano
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
1 sibling, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-05-25 9:15 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> Emit a "did you mean ..." hint suggesting the split form. Restrict it
> to plausible-set contexts ("git config set", bare "git config <key>",
> and their 2-arg forms); explicit "get"/"unset" keep the existing error.
I understand that it would be a good idea to give this warning
against these two where $A is an arbitrary string with at least one
dot in it (making it a likely variable name), and $B is an arbitrary
string that may contain anything:
git config set "$A=$B"
git config "$A=$B"
It is plausible that the user wanted to make the value of the
variable "$A" to "$B", so telling them the right syntax would be
valuable.
If "$A" is a syntactically valid variable name, then I would imagine
that we want to say something like this:
$ git config set "$A=$B"
error: missing value to set to the variable "$A=$B"
hint: did you mean 'git config set "$A" "$B"'?
If "$A" is *not* a syntactically valid variable name, then giving a
hint to try to assing to it is a counter-productive. Ideally we
probably want something like:
$ git config set "foo=bar"
error: missing value to set to a variable with an invalid name 'foo=bar'
It is pointless to say the user may have meant "git config set foo bar",
as "foo" is clearly not a valid variable.
I do not understand what you mean by "their 2-arg forms". Do you
mean
git config set "$A=$B" "$C"
by that? If so, I doubt that user meant an assignment to "$A" by
this form with explicit "set". If "$A=$B" is a variable whose name
is valid (i.e. three-level name whose the second level component
contains a "="), we should just take it as asked. E.g.,
git config set "foo.bar=baz.boo" "some-string"
needs no hand holding. But
if "$A=$B" is not a valid variable name, we should just complain
that the user is trying to assign to a variable with an invalid
name.
$ git config set "foo.bar=baz" "some-string"
error: setting to a variable with invalid name 'foo.bar=baz'
I think
git config "$A=$B" "$C"
that implicitly uses the 'set' verb can be left as an exercise to
readers. If "$A=$B" is a valid name, we shouldn't do any complaint.
If it is not,
$ git config "foo.bar=baz" "some-string"
error: setting to a variable with invalid name 'foo.bar=baz'
It makes it clear to the user that (1) we interpreted the command
line to be "implicit set", (2) we interpreted the command line to
set variable 'foo.bar=baz', and (3) 'foo.bar=baz' is not a valid
name. I do not think there is anything more needed for this case.
> "=" is legal inside a subsection, so only fire when "=" lands after
> the last ".". When the user supplied a separate value, use it in the
> suggestion instead of the suffix after "=":
>
> $ git config set pull.rebase=false true
> error: invalid key: pull.rebase=false
> hint: did you mean "git config set pull.rebase true"?
I really do not think '=' needs *any* special casing in this case.
If we used "pull.rebase*false" as the variable instead, the message
would say that "pull.rebase*false" is an invalid key. Two important
things for this message to convey are (1) the command correctly
parsed the command line to mean that the user wants to assign to a
variable whose name is 'pull.rebase*false' and (2) that variable
name *is* invalid.
If you find the current message suboptimal, I think we should try to
clarify the message, as '=' or '*' or any letter that makes the
variable name invalid would benefit from the same improvement.
Perhaps something like:
$ git config set pull.rebase*false true
error: setting to a variable with invalid name: 'pull.rebase*false'
perhaps?
> Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
Interesting. We typically do not do this.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4] config: improve diagnostic for "set" with missing value
2026-05-25 8:33 ` [PATCH v3] " Harald Nordgren via GitGitGadget
2026-05-25 9:15 ` Junio C Hamano
@ 2026-05-26 19:21 ` Harald Nordgren via GitGitGadget
2026-05-26 19:24 ` Harald Nordgren
` (3 more replies)
1 sibling, 4 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-05-26 19:21 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
config: suggest the correct form when key contains "="
* Skip the hint when the inferred value contains whitespace, so git
config set pull.rebase=false "hello world" no longer suggests a
malformed command.
* Replace the inline actions == 0 check with a named actions_implicit
flag, simplfied the code.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v4
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v3:
1: 6b9d66361d ! 1: 780b99409c config: suggest the correct form when key contains "=" in set context
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail•com>
## Commit message ##
- config: suggest the correct form when key contains "=" in set context
+ config: improve diagnostic for "set" with missing value
- A user who types "git config pull.rebase=false" gets only "error:
- invalid key: pull.rebase=false" with no clue what went wrong.
+ "git config set pull.rebase=false" currently fails with "wrong
+ number of arguments", and the implicit form "git config
+ pull.rebase=false" fails with "invalid key". Neither points at
+ the real problem: the value is missing.
- Emit a "did you mean ..." hint suggesting the split form. Restrict it
- to plausible-set contexts ("git config set", bare "git config <key>",
- and their 2-arg forms); explicit "get"/"unset" keep the existing error.
+ Report that directly, and when the argument has the shape
+ "<valid-key>=<value>", also suggest the split form:
- "=" is legal inside a subsection, so only fire when "=" lands after
- the last ".". When the user supplied a separate value, use it in the
- suggestion instead of the suffix after "=":
+ $ git config set pull.rebase=false
+ error: missing value to set to the variable 'pull.rebase=false'
+ hint: did you mean "git config set pull.rebase false"?
- $ git config set pull.rebase=false true
- error: invalid key: pull.rebase=false
- hint: did you mean "git config set pull.rebase true"?
+ When the prefix before "=" is not a valid key, drop the hint:
+
+ $ git config set foo=bar
+ error: missing value to set to a variable with an invalid name 'foo=bar'
- Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
## builtin/config.c ##
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
exit(129);
}
-+static void advise_setting_with_equals(const char *key, const char *value)
++static int is_valid_key(const char *key)
+{
+ const char *last_dot = strrchr(key, '.');
-+ const char *eq;
+
-+ if (!last_dot)
-+ return;
-+ eq = strchr(last_dot + 1, '=');
-+ if (!eq)
-+ return;
-+ if (!value)
-+ value = eq + 1;
-+ if (!*value || strpbrk(value, " \t\n"))
-+ return;
-+ advise(_("did you mean \"git config set %.*s %s\"?"),
-+ (int)(eq - key), key, value);
++ return last_dot && isalpha(last_dot[1]);
++}
++
++static NORETURN void die_missing_set_value(const char *arg)
++{
++ const char *last_dot = strrchr(arg, '.');
++ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
++ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
++
++ if (prefix && is_valid_key(prefix)) {
++ error(_("missing value to set to the variable '%s'"), arg);
++ advise(_("did you mean \"git config set %s %s\"?"),
++ prefix, eq + 1);
++ } else if (is_valid_key(arg)) {
++ error(_("missing value to set to the variable '%s'"), arg);
++ } else {
++ error(_("missing value to set to a variable with an invalid name '%s'"),
++ arg);
++ }
++ free(prefix);
++ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const c
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
-+ if (argc == 1 && strchr(argv[0], '=')) {
-+ error(_("wrong number of arguments, should be 2"));
-+ advise_setting_with_equals(argv[0], NULL);
-+ exit(129);
-+ }
++ if (argc == 1)
++ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
-@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const char *prefix,
- error(_("cannot overwrite multiple values with a single value\n"
- " Use --value=<pattern>, --append or --all to change %s."), argv[0]);
- }
-+ if (ret == CONFIG_INVALID_KEY)
-+ advise_setting_with_equals(argv[0], argv[1]);
-
- location_options_release(&location_opts);
- free(comment);
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
- if (ret == CONFIG_NOTHING_SET)
- error(_("cannot overwrite multiple values with a single value\n"
- " Use a regexp, --add or --replace-all to change %s."), argv[0]);
-+ else if (ret == CONFIG_INVALID_KEY)
-+ advise_setting_with_equals(argv[0], argv[1]);
- }
- else if (actions == ACTION_SET_ALL) {
- check_write(&location_opts.source);
-@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
- check_argc(argc, 1, 2);
- ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
- 0, flags);
-+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
-+ advise_setting_with_equals(argv[0], NULL);
- }
- else if (actions == ACTION_GET_ALL) {
- check_argc(argc, 1, 2);
+ error(_("no action specified"));
+ exit(129);
+ }
++ if (actions_implicit && argc == 1) {
++ const char *last_dot = strrchr(argv[0], '.');
++ if (last_dot && strchr(last_dot + 1, '='))
++ die_missing_set_value(argv[0]);
++ }
+ if (display_opts.omit_values &&
+ !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
+ error(_("--name-only is only applicable to --list or --get-regexp"));
## t/t1300-config.sh ##
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
-+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
-+ test_must_fail git config pull.rebase=false 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
++ test_must_fail git config set pull.rebase=false 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
-+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
-+ test_must_fail git config pull.rebase=false true 2>err &&
-+ test_grep "did you mean .git config set pull\\.rebase true." err
-+'
-+
-+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
-+ test_must_fail git config set pull.rebase=false true 2>err &&
-+ test_grep "did you mean .git config set pull\\.rebase true." err
-+'
-+
-+test_expect_success 'misplaced "=" in key: set with single arg hints' '
-+ test_must_fail git config set pull.rebase=false 2>err &&
-+ test_grep "wrong number of arguments" err &&
++test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
++ test_must_fail git config pull.rebase=false 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
-+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
-+ test_must_fail git config --get pull.rebase=false 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
++ test_must_fail git config set foo=bar 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
-+ test_must_fail git config get pull.rebase=false 2>err &&
++test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
++ test_must_fail git config set foo.1bar=baz 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
-+ test_must_fail git config unset pull.rebase=false 2>err &&
++test_expect_success 'set with 1 arg of valid key reports missing value' '
++ test_must_fail git config set pull.rebase 2>err &&
++ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
-+ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
-+ test_grep "invalid key: pull\\.rebase=false" err &&
++test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
++ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
-+test_expect_success '"=" inside subsection is valid, no hint' '
++test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
-+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
-+ test_grep ! "did you mean" err &&
++ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
builtin/config.c | 39 ++++++++++++++++++++++++++++++++++++++-
t/t1300-config.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..6fe2d85814 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,33 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static int is_valid_key(const char *key)
+{
+ const char *last_dot = strrchr(key, '.');
+
+ return last_dot && isalpha(last_dot[1]);
+}
+
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && is_valid_key(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (is_valid_key(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1161,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1401,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1416,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1426,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..4a8a381bd8 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,49 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
base-commit: 56a4f3c3a221adf1df9b39da69b8a6890f803157
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
@ 2026-05-26 19:24 ` Harald Nordgren
2026-06-01 23:45 ` Junio C Hamano
` (2 subsequent siblings)
3 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren @ 2026-05-26 19:24 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Kristoffer Haugsbakk
I forgot to update the PR description on GitHub, it should have read:
- Diagnose the 1-arg set form (explicit and implicit) directly: report
the missing value, and suggest the split form only when the prefix
before `=` is a valid key.
- Did not act on Junio's secondary suggestion to reword the 2-arg
`error: invalid key: <key>`, fix seemed to become too big.
Harald
On Tue, May 26, 2026 at 9:21 PM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail•com> wrote:
>
> From: Harald Nordgren <haraldnordgren@gmail•com>
>
> "git config set pull.rebase=false" currently fails with "wrong
> number of arguments", and the implicit form "git config
> pull.rebase=false" fails with "invalid key". Neither points at
> the real problem: the value is missing.
>
> Report that directly, and when the argument has the shape
> "<valid-key>=<value>", also suggest the split form:
>
> $ git config set pull.rebase=false
> error: missing value to set to the variable 'pull.rebase=false'
> hint: did you mean "git config set pull.rebase false"?
>
> When the prefix before "=" is not a valid key, drop the hint:
>
> $ git config set foo=bar
> error: missing value to set to a variable with an invalid name 'foo=bar'
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
> ---
> config: suggest the correct form when key contains "="
>
> * Skip the hint when the inferred value contains whitespace, so git
> config set pull.rebase=false "hello world" no longer suggests a
> malformed command.
> * Replace the inline actions == 0 check with a named actions_implicit
> flag, simplfied the code.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v4
> Pull-Request: https://github.com/git/git/pull/2302
>
> Range-diff vs v3:
>
> 1: 6b9d66361d ! 1: 780b99409c config: suggest the correct form when key contains "=" in set context
> @@ Metadata
> Author: Harald Nordgren <haraldnordgren@gmail•com>
>
> ## Commit message ##
> - config: suggest the correct form when key contains "=" in set context
> + config: improve diagnostic for "set" with missing value
>
> - A user who types "git config pull.rebase=false" gets only "error:
> - invalid key: pull.rebase=false" with no clue what went wrong.
> + "git config set pull.rebase=false" currently fails with "wrong
> + number of arguments", and the implicit form "git config
> + pull.rebase=false" fails with "invalid key". Neither points at
> + the real problem: the value is missing.
>
> - Emit a "did you mean ..." hint suggesting the split form. Restrict it
> - to plausible-set contexts ("git config set", bare "git config <key>",
> - and their 2-arg forms); explicit "get"/"unset" keep the existing error.
> + Report that directly, and when the argument has the shape
> + "<valid-key>=<value>", also suggest the split form:
>
> - "=" is legal inside a subsection, so only fire when "=" lands after
> - the last ".". When the user supplied a separate value, use it in the
> - suggestion instead of the suffix after "=":
> + $ git config set pull.rebase=false
> + error: missing value to set to the variable 'pull.rebase=false'
> + hint: did you mean "git config set pull.rebase false"?
>
> - $ git config set pull.rebase=false true
> - error: invalid key: pull.rebase=false
> - hint: did you mean "git config set pull.rebase true"?
> + When the prefix before "=" is not a valid key, drop the hint:
> +
> + $ git config set foo=bar
> + error: missing value to set to a variable with an invalid name 'foo=bar'
>
> - Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn•se>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
>
> ## builtin/config.c ##
> @@ builtin/config.c: static void check_argc(int argc, int min, int max)
> exit(129);
> }
>
> -+static void advise_setting_with_equals(const char *key, const char *value)
> ++static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> -+ const char *eq;
> +
> -+ if (!last_dot)
> -+ return;
> -+ eq = strchr(last_dot + 1, '=');
> -+ if (!eq)
> -+ return;
> -+ if (!value)
> -+ value = eq + 1;
> -+ if (!*value || strpbrk(value, " \t\n"))
> -+ return;
> -+ advise(_("did you mean \"git config set %.*s %s\"?"),
> -+ (int)(eq - key), key, value);
> ++ return last_dot && isalpha(last_dot[1]);
> ++}
> ++
> ++static NORETURN void die_missing_set_value(const char *arg)
> ++{
> ++ const char *last_dot = strrchr(arg, '.');
> ++ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
> ++ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
> ++
> ++ if (prefix && is_valid_key(prefix)) {
> ++ error(_("missing value to set to the variable '%s'"), arg);
> ++ advise(_("did you mean \"git config set %s %s\"?"),
> ++ prefix, eq + 1);
> ++ } else if (is_valid_key(arg)) {
> ++ error(_("missing value to set to the variable '%s'"), arg);
> ++ } else {
> ++ error(_("missing value to set to a variable with an invalid name '%s'"),
> ++ arg);
> ++ }
> ++ free(prefix);
> ++ exit(129);
> +}
> +
> static void show_config_origin(const struct config_display_options *opts,
> @@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const c
>
> argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> -+ if (argc == 1 && strchr(argv[0], '=')) {
> -+ error(_("wrong number of arguments, should be 2"));
> -+ advise_setting_with_equals(argv[0], NULL);
> -+ exit(129);
> -+ }
> ++ if (argc == 1)
> ++ die_missing_set_value(argv[0]);
> check_argc(argc, 2, 2);
>
> if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
> -@@ builtin/config.c: static int cmd_config_set(int argc, const char **argv, const char *prefix,
> - error(_("cannot overwrite multiple values with a single value\n"
> - " Use --value=<pattern>, --append or --all to change %s."), argv[0]);
> - }
> -+ if (ret == CONFIG_INVALID_KEY)
> -+ advise_setting_with_equals(argv[0], argv[1]);
> -
> - location_options_release(&location_opts);
> - free(comment);
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> };
> char *value = NULL, *comment = NULL;
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, con
> case 1: actions = ACTION_GET; break;
> case 2: actions = ACTION_SET; break;
> @@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> - if (ret == CONFIG_NOTHING_SET)
> - error(_("cannot overwrite multiple values with a single value\n"
> - " Use a regexp, --add or --replace-all to change %s."), argv[0]);
> -+ else if (ret == CONFIG_INVALID_KEY)
> -+ advise_setting_with_equals(argv[0], argv[1]);
> - }
> - else if (actions == ACTION_SET_ALL) {
> - check_write(&location_opts.source);
> -@@ builtin/config.c: static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> - check_argc(argc, 1, 2);
> - ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
> - 0, flags);
> -+ if (ret == CONFIG_INVALID_KEY && actions_implicit)
> -+ advise_setting_with_equals(argv[0], NULL);
> - }
> - else if (actions == ACTION_GET_ALL) {
> - check_argc(argc, 1, 2);
> + error(_("no action specified"));
> + exit(129);
> + }
> ++ if (actions_implicit && argc == 1) {
> ++ const char *last_dot = strrchr(argv[0], '.');
> ++ if (last_dot && strchr(last_dot + 1, '='))
> ++ die_missing_set_value(argv[0]);
> ++ }
> + if (display_opts.omit_values &&
> + !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
> + error(_("--name-only is only applicable to --list or --get-regexp"));
>
> ## t/t1300-config.sh ##
> @@ t/t1300-config.sh: test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> -+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
> -+ test_must_fail git config pull.rebase=false 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> ++ test_must_fail git config set pull.rebase=false 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
> -+ test_must_fail git config pull.rebase=false true 2>err &&
> -+ test_grep "did you mean .git config set pull\\.rebase true." err
> -+'
> -+
> -+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
> -+ test_must_fail git config set pull.rebase=false true 2>err &&
> -+ test_grep "did you mean .git config set pull\\.rebase true." err
> -+'
> -+
> -+test_expect_success 'misplaced "=" in key: set with single arg hints' '
> -+ test_must_fail git config set pull.rebase=false 2>err &&
> -+ test_grep "wrong number of arguments" err &&
> ++test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> ++ test_must_fail git config pull.rebase=false 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
> -+ test_must_fail git config --get pull.rebase=false 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> ++ test_must_fail git config set foo=bar 2>err &&
> ++ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
> -+ test_must_fail git config get pull.rebase=false 2>err &&
> ++test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> ++ test_must_fail git config set foo.1bar=baz 2>err &&
> ++ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
> -+ test_must_fail git config unset pull.rebase=false 2>err &&
> ++test_expect_success 'set with 1 arg of valid key reports missing value' '
> ++ test_must_fail git config set pull.rebase 2>err &&
> ++ test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
> -+ test_must_fail git config set pull.rebase=false "hello world" 2>err &&
> -+ test_grep "invalid key: pull\\.rebase=false" err &&
> ++test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> ++ test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
> +
> -+test_expect_success '"=" inside subsection is valid, no hint' '
> ++test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> -+ git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
> -+ test_grep ! "did you mean" err &&
> ++ git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
>
>
> builtin/config.c | 39 ++++++++++++++++++++++++++++++++++++++-
> t/t1300-config.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index cf4ba0f7cc..6fe2d85814 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -1,6 +1,7 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "abspath.h"
> +#include "advice.h"
> #include "config.h"
> #include "color.h"
> #include "date.h"
> @@ -210,6 +211,33 @@ static void check_argc(int argc, int min, int max)
> exit(129);
> }
>
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
> +
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
> +
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
> + free(prefix);
> + exit(129);
> +}
> +
> static void show_config_origin(const struct config_display_options *opts,
> const struct key_value_info *kvi,
> struct strbuf *buf)
> @@ -1133,6 +1161,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
>
> argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> + if (argc == 1)
> + die_missing_set_value(argv[0]);
> check_argc(argc, 2, 2);
>
> if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
> @@ -1371,6 +1401,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> };
> char *value = NULL, *comment = NULL;
> int ret = 0;
> + int actions_implicit;
> struct key_value_info default_kvi = KVI_INIT;
>
> argc = parse_options(argc, argv, prefix, opts,
> @@ -1385,7 +1416,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> exit(129);
> }
>
> - if (actions == 0)
> + actions_implicit = (actions == 0);
> + if (actions_implicit)
> switch (argc) {
> case 1: actions = ACTION_GET; break;
> case 2: actions = ACTION_SET; break;
> @@ -1394,6 +1426,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
> error(_("no action specified"));
> exit(129);
> }
> + if (actions_implicit && argc == 1) {
> + const char *last_dot = strrchr(argv[0], '.');
> + if (last_dot && strchr(last_dot + 1, '='))
> + die_missing_set_value(argv[0]);
> + }
> if (display_opts.omit_values &&
> !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
> error(_("--name-only is only applicable to --list or --get-regexp"));
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..4a8a381bd8 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,49 @@ test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> + test_must_fail git config set pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> + test_must_fail git config pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
> +
> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> + test_must_fail git config set foo=bar 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> + test_must_fail git config set foo.1bar=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> + test_must_fail git config set pull.rebase 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> + test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
> +
> +test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> + git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'correct key' '
> git config 123456.a123 987
> '
>
> base-commit: 56a4f3c3a221adf1df9b39da69b8a6890f803157
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-05-26 19:24 ` Harald Nordgren
@ 2026-06-01 23:45 ` Junio C Hamano
2026-06-01 23:53 ` Junio C Hamano
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
3 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-06-01 23:45 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
None of these are valid configuration variable names, but this
function would allow any of them, no?
1foo.bar
1foo.some.bar
foo.b_r
foo.some.b_r
or does the caller reject such "key" before calling us?
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
OK, the intention is to see "foo.bar=baz" and guess that assinging
to "foo.bar" might be what the user wanted. eq here would point at
that '='. And ...
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
... prefix is our own copy of "foo.bar".
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
OK. If is_valid_key() rejected invalid variable names correctly,
this would catch $A=$B where $A is a plausible-looking name.
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
The distinction among these three messages does look reasonable,
provided if is_valid_key() gives the correct result.
I wonder if it is too hard to refactor existing logic (perhaps it is
used in git_config_parse_key(), no?) to give us a less noisy version
of it that we can use as is_valid_key() here?
Other than that, the remainder of the code changes looked reasonable
to me. Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4] config: improve diagnostic for "set" with missing value
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-05-26 19:24 ` Harald Nordgren
2026-06-01 23:45 ` Junio C Hamano
@ 2026-06-01 23:53 ` Junio C Hamano
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
3 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-06-01 23:53 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> +static int is_valid_key(const char *key)
> +{
> + const char *last_dot = strrchr(key, '.');
> +
> + return last_dot && isalpha(last_dot[1]);
> +}
None of these are valid configuration variable names, but this
function would allow any of them, no?
1foo.bar
1foo.some.bar
foo.b_r
foo.some.b_r
or does the caller reject such "key" before calling us?
> +static NORETURN void die_missing_set_value(const char *arg)
> +{
> + const char *last_dot = strrchr(arg, '.');
> + const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
OK, the intention is to see "foo.bar=baz" and guess that assinging
to "foo.bar" might be what the user wanted. eq here would point at
that '='. And ...
> + char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
... prefix is our own copy of "foo.bar".
> + if (prefix && is_valid_key(prefix)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + advise(_("did you mean \"git config set %s %s\"?"),
> + prefix, eq + 1);
OK. If is_valid_key() rejected invalid variable names correctly,
this would catch $A=$B where $A is a plausible-looking name.
> + } else if (is_valid_key(arg)) {
> + error(_("missing value to set to the variable '%s'"), arg);
> + } else {
> + error(_("missing value to set to a variable with an invalid name '%s'"),
> + arg);
> + }
The distinction among these three messages does look reasonable,
provided if is_valid_key() gives the correct result.
I wonder if it is too hard to refactor existing logic (perhaps it is
used in git_config_parse_key(), no?) to give us a less noisy version
of it that we can use as is_valid_key() here?
Other than that, the remainder of the code changes looked reasonable
to me. Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5 0/2] config: suggest the correct form when key contains "="
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
` (2 preceding siblings ...)
2026-06-01 23:53 ` Junio C Hamano
@ 2026-06-02 13:39 ` Harald Nordgren via GitGitGadget
2026-06-02 13:39 ` [PATCH v5 1/2] config: let git_config_parse_key() validate quietly Harald Nordgren via GitGitGadget
` (2 more replies)
3 siblings, 3 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren
* New commit config: let git_config_parse_key() validate quietly adds a
quiet parameter (and an optional store_key) so callers can validate
without writing to stderr.
* Validation in die_missing_set_value() now routes through
git_config_parse_key(key, NULL, NULL, 1) instead of the previous local
helper.
* Added tests for 1foo.bar=baz and foo.some.b_r=baz.
Harald Nordgren (2):
config: let git_config_parse_key() validate quietly
config: improve diagnostic for "set" with missing value
builtin/config.c | 34 ++++++++++++++++++++++++++--
config.c | 34 ++++++++++++++++++----------
config.h | 2 +-
submodule-config.c | 2 +-
t/t1300-config.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 111 insertions(+), 16 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v5
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v4:
-: ---------- > 1: d938ebf95a config: let git_config_parse_key() validate quietly
1: 780b99409c ! 2: e5a2070ee1 config: improve diagnostic for "set" with missing value
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
exit(129);
}
-+static int is_valid_key(const char *key)
-+{
-+ const char *last_dot = strrchr(key, '.');
-+
-+ return last_dot && isalpha(last_dot[1]);
-+}
-+
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
-+ if (prefix && is_valid_key(prefix)) {
++ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
-+ } else if (is_valid_key(arg)) {
++ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+ test_grep ! "did you mean" err
+'
+
++test_expect_success 'set with 1 arg: digit-led section name is valid' '
++ test_must_fail git config set 1foo.bar=baz 2>err &&
++ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
++ test_grep "did you mean .git config set 1foo\\.bar baz." err
++'
++
++test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
++ test_must_fail git config set foo.some.b_r=baz 2>err &&
++ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
++ test_grep ! "did you mean" err
++'
++
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
--
gitgitgadget
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
@ 2026-06-02 13:39 ` Harald Nordgren via GitGitGadget
2026-06-02 14:08 ` Junio C Hamano
2026-06-02 13:39 ` [PATCH v5 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2 siblings, 1 reply; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
Add a "quiet" parameter that suppresses the error() calls, and let
store_key be NULL to skip the canonical-copy allocation. Existing
callers pass 0 for quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
builtin/config.c | 2 +-
config.c | 34 ++++++++++++++++++++++------------
config.h | 2 +-
submodule-config.c | 2 +-
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..b3188cd8d4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -555,7 +555,7 @@ static int get_value(const struct config_location_options *opts,
goto free_strings;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
+ if (git_config_parse_key(key_, &key, NULL, 0)) {
ret = CONFIG_INVALID_KEY;
goto free_strings;
}
diff --git a/config.c b/config.c
index a1b92fe083..81b31c5155 100644
--- a/config.c
+++ b/config.c
@@ -536,11 +536,14 @@ static inline int iskeychar(int c)
* -2 if there is no section name in the key.
*
* store_key - pointer to char* which will hold a copy of the key with
- * lowercase section and variable name
+ * lowercase section and variable name, can be NULL to skip
+ * allocation when only validation is needed
* baselen - pointer to size_t which will hold the length of the
* section + subsection part, can be NULL
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
+ int quiet)
{
size_t i, baselen;
int dot;
@@ -552,12 +555,14 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(_("key does not contain a section: %s"), key);
+ if (!quiet)
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(_("key does not contain variable name: %s"), key);
+ if (!quiet)
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -568,7 +573,8 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
/*
* Validate the key and while at it, lower case it for matching.
*/
- *store_key = xmallocz(strlen(key));
+ if (store_key)
+ *store_key = xmallocz(strlen(key));
dot = 0;
for (i = 0; key[i]; i++) {
@@ -579,21 +585,25 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!dot || i > baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
- error(_("invalid key: %s"), key);
+ if (!quiet)
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(_("invalid key (newline): %s"), key);
+ if (!quiet)
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
- (*store_key)[i] = c;
+ if (store_key)
+ (*store_key)[i] = c;
}
return 0;
out_free_ret_1:
- FREE_AND_NULL(*store_key);
+ if (store_key)
+ FREE_AND_NULL(*store_key);
return -CONFIG_INVALID_KEY;
}
@@ -609,7 +619,7 @@ static int config_parse_pair(const char *key, const char *value,
if (!strlen(key))
return error(_("empty config key"));
- if (git_config_parse_key(key, &canonical_name, NULL))
+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
return -1;
ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
@@ -1708,7 +1718,7 @@ static int configset_find_element(struct config_set *set, const char *key,
* `key` may come from the user, so normalize it before using it
* for querying entries from the hashmap.
*/
- ret = git_config_parse_key(key, &normalized_key, NULL);
+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
if (ret)
return ret;
@@ -3001,7 +3011,7 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
validate_comment_string(comment);
/* parse-key returns negative; flip the sign to feed exit(3) */
- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
if (ret)
goto out_free;
diff --git a/config.h b/config.h
index bf47fb3afc..2c66d334c1 100644
--- a/config.h
+++ b/config.h
@@ -341,7 +341,7 @@ int repo_config_set_worktree_gently(struct repository *, const char *, const cha
*/
void repo_config_set(struct repository *, const char *, const char *);
-int git_config_parse_key(const char *, char **, size_t *);
+int git_config_parse_key(const char *, char **, size_t *, int quiet);
/*
* The following macros specify flag bits that alter the behavior
diff --git a/submodule-config.c b/submodule-config.c
index a81897b4e0..a319956f7a 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -970,7 +970,7 @@ int print_config_from_gitmodules(struct repository *repo, const char *key)
int ret;
char *store_key;
- ret = git_config_parse_key(key, &store_key, NULL);
+ ret = git_config_parse_key(key, &store_key, NULL, 0);
if (ret < 0)
return CONFIG_INVALID_KEY;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 13:39 ` [PATCH v5 1/2] config: let git_config_parse_key() validate quietly Harald Nordgren via GitGitGadget
@ 2026-06-02 13:39 ` Harald Nordgren via GitGitGadget
2026-06-02 14:18 ` Junio C Hamano
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2 siblings, 1 reply; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 13:39 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
builtin/config.c | 32 ++++++++++++++++++++++++++-
t/t1300-config.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index b3188cd8d4..a2d46d0ce1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,26 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1154,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1394,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1409,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1419,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..ed122d1100 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: digit-led section name is valid' '
+ test_must_fail git config set 1foo.bar=baz 2>err &&
+ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
+ test_grep "did you mean .git config set 1foo\\.bar baz." err
+'
+
+test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
+ test_must_fail git config set foo.some.b_r=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
2026-06-02 13:39 ` [PATCH v5 1/2] config: let git_config_parse_key() validate quietly Harald Nordgren via GitGitGadget
@ 2026-06-02 14:08 ` Junio C Hamano
2026-06-02 16:31 ` Harald Nordgren
0 siblings, 1 reply; 36+ messages in thread
From: Junio C Hamano @ 2026-06-02 14:08 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> From: Harald Nordgren <haraldnordgren@gmail•com>
>
> Add a "quiet" parameter that suppresses the error() calls, and let
> store_key be NULL to skip the canonical-copy allocation. Existing
> callers pass 0 for quiet.
Hmph.
The way this patch did this may have been easier to implement, but
is a bit different from what I had in mind when I suggested to
"refactor" the existing logic.
Perhaps the updated "git_config_parse_key()" in this patch should be
renamed to be a file-scape static internal helper, and the existing
"git_config_parse_key()" should become a thin wrapper around that
new helper function, retaining the current external interface,
requiring no changes to existing callers.
Then in the next step, config.[ch] can add a new entry point that
serves the purpose of is_valid_key() in the previous iteration,
perhaps call it is_valid_git_config_key() or something like that
(Patrick or others may want to suggest a better word order in its
name). That way, we do not have to sprinkle many calls into
this (rather ugly) version of git_config_parse_key() with overly
wide interface that repeats meaningless NULL/0/1 parameters that no
callers want to use (other than for the purpose of differenciating
the real git_config_parse_key() calls from the new calls made to the
same function to ask "is this a valid key or not, yes/no?".
Thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5 2/2] config: improve diagnostic for "set" with missing value
2026-06-02 13:39 ` [PATCH v5 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
@ 2026-06-02 14:18 ` Junio C Hamano
0 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-06-02 14:18 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..ed122d1100 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
> test_must_fail git config inval.2key blabla
> '
>
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> + test_must_fail git config set pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
This is a syntax error of the command line, but the lhs of '=' makes
us suspect that the user may have meant to assign to that variable.
Makes perfect sense.
> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> + test_must_fail git config pull.rebase=false 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> + test_grep "did you mean .git config set pull\\.rebase false." err
> +'
Ditto, the syntax may be an implicit "get" with bogus variable name,
or an implicit "set" with variable name and its value concatenated
into one argument with '='. The message seems to be assuming the
latter, which is OK to me.
> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> + test_must_fail git config set foo=bar 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> + test_grep ! "did you mean" err
> +'
OK.
> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> + test_must_fail git config set foo.1bar=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> + test_grep ! "did you mean" err
> +'
OK. The above two should always give us the same error (except for
the actual bogus names given by the user to the command).
> +test_expect_success 'set with 1 arg: digit-led section name is valid' '
> + test_must_fail git config set 1foo.bar=baz 2>err &&
> + test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
> + test_grep "did you mean .git config set 1foo\\.bar baz." err
> +'
OK.
> +test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
> + test_must_fail git config set foo.some.b_r=baz 2>err &&
> + test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
> + test_grep ! "did you mean" err
> +'
This is the third one that should be identical to earlier two that
gave a bogus variable name.
> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> + test_must_fail git config set pull.rebase 2>err &&
> + test_grep "missing value to set to the variable .pull\\.rebase." err &&
> + test_grep ! "did you mean" err
> +'
Did we see this already? No, this is different from the earlier one
that had "=false". This is a bog standard "you said set but did not
say what value to set to". Good.
> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> + test_must_fail git config set pull.rebase=false true 2>err &&
> + test_grep ! "did you mean" err
> +'
OK. Do we want to see that the bogus variable name reported?
> +test_expect_success '"=" inside subsection is valid' '
> + test_when_finished "rm -f subsection.cfg" &&
> + git config set -f subsection.cfg foo.bar=baz.boo qux &&
> + echo qux >expect &&
> + git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> + test_cmp expect actual
> +'
Excellent.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
2026-06-02 14:08 ` Junio C Hamano
@ 2026-06-02 16:31 ` Harald Nordgren
2026-06-04 1:09 ` Junio C Hamano
0 siblings, 1 reply; 36+ messages in thread
From: Harald Nordgren @ 2026-06-02 16:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk
> Perhaps the updated "git_config_parse_key()" in this patch should be
> renamed to be a file-scape static internal helper, and the existing
> "git_config_parse_key()" should become a thin wrapper around that
> new helper function, retaining the current external interface,
> requiring no changes to existing callers.
I want to remember a discussion on one of my earlier topics, a few
months back, where someone else suggested instead of introducing two
thin wrappers over a helper, we should update the callers instead.
But for me either way is fine, maybe here it makes more sense, because
of the repeated NULL/0/1 parameters.
Harald
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v6 0/2] config: suggest the correct form when key contains "="
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 13:39 ` [PATCH v5 1/2] config: let git_config_parse_key() validate quietly Harald Nordgren via GitGitGadget
2026-06-02 13:39 ` [PATCH v5 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
@ 2026-06-02 18:43 ` Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation Harald Nordgren via GitGitGadget
` (2 more replies)
2 siblings, 3 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren
* The quiet parameter now lives on a static do_parse_config_key() instead
of git_config_parse_key() itself. git_config_parse_key() is back to its
three-argument signature; existing callers don't change.
* New public git_config_key_is_valid() for callers that only need a yes/no
check.
Harald Nordgren (2):
config: add git_config_key_is_valid() for quiet validation
config: improve diagnostic for "set" with missing value
builtin/config.c | 32 ++++++++++++++++++++++++++-
config.c | 38 ++++++++++++++++++++++++--------
config.h | 2 ++
t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 118 insertions(+), 10 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v6
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v5:
1: d938ebf95a ! 1: 7400ca41bb config: let git_config_parse_key() validate quietly
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail•com>
## Commit message ##
- config: let git_config_parse_key() validate quietly
+ config: add git_config_key_is_valid() for quiet validation
- Add a "quiet" parameter that suppresses the error() calls, and let
- store_key be NULL to skip the canonical-copy allocation. Existing
- callers pass 0 for quiet.
+ Move the body of git_config_parse_key() into a static helper
+ do_parse_config_key() that takes a "quiet" flag and treats
+ store_key as optional. git_config_parse_key() becomes a thin
+ wrapper.
- Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
+ Add git_config_key_is_valid() for callers that only need to
+ know whether a key is well-formed.
- ## builtin/config.c ##
-@@ builtin/config.c: static int get_value(const struct config_location_options *opts,
- goto free_strings;
- }
- } else {
-- if (git_config_parse_key(key_, &key, NULL)) {
-+ if (git_config_parse_key(key_, &key, NULL, 0)) {
- ret = CONFIG_INVALID_KEY;
- goto free_strings;
- }
+ Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
## config.c ##
@@ config.c: static inline int iskeychar(int c)
@@ config.c: static inline int iskeychar(int c)
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
-+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
-+ int quiet)
++static int do_parse_config_key(const char *key, char **store_key,
++ size_t *baselen_, int quiet)
{
size_t i, baselen;
int dot;
@@ config.c: int git_config_parse_key(const char *key, char **store_key, size_t *ba
return -CONFIG_INVALID_KEY;
}
-@@ config.c: static int config_parse_pair(const char *key, const char *value,
-
- if (!strlen(key))
- return error(_("empty config key"));
-- if (git_config_parse_key(key, &canonical_name, NULL))
-+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
- return -1;
-
- ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
-@@ config.c: static int configset_find_element(struct config_set *set, const char *key,
- * `key` may come from the user, so normalize it before using it
- * for querying entries from the hashmap.
- */
-- ret = git_config_parse_key(key, &normalized_key, NULL);
-+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
- if (ret)
- return ret;
-
-@@ config.c: int repo_config_set_multivar_in_file_gently(struct repository *r,
- validate_comment_string(comment);
-
- /* parse-key returns negative; flip the sign to feed exit(3) */
-- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
-+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
- if (ret)
- goto out_free;
-
++int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
++{
++ return do_parse_config_key(key, store_key, baselen_, 0);
++}
++
++int git_config_key_is_valid(const char *key)
++{
++ return !do_parse_config_key(key, NULL, NULL, 1);
++}
++
+ static int config_parse_pair(const char *key, const char *value,
+ struct key_value_info *kvi,
+ config_fn_t fn, void *data)
## config.h ##
-@@ config.h: int repo_config_set_worktree_gently(struct repository *, const char *, const cha
- */
- void repo_config_set(struct repository *, const char *, const char *);
+@@ config.h: void repo_config_set(struct repository *, const char *, const char *);
--int git_config_parse_key(const char *, char **, size_t *);
-+int git_config_parse_key(const char *, char **, size_t *, int quiet);
+ int git_config_parse_key(const char *, char **, size_t *);
++int git_config_key_is_valid(const char *);
++
/*
* The following macros specify flag bits that alter the behavior
-
- ## submodule-config.c ##
-@@ submodule-config.c: int print_config_from_gitmodules(struct repository *repo, const char *key)
- int ret;
- char *store_key;
-
-- ret = git_config_parse_key(key, &store_key, NULL);
-+ ret = git_config_parse_key(key, &store_key, NULL, 0);
- if (ret < 0)
- return CONFIG_INVALID_KEY;
-
+ * of the repo_config_set_multivar*() methods.
2: e5a2070ee1 ! 2: a7f8a084c7 config: improve diagnostic for "set" with missing value
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
-+ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
++ if (prefix && git_config_key_is_valid(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
-+ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
++ } else if (git_config_key_is_valid(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
++ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
--
gitgitgadget
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
@ 2026-06-02 18:43 ` Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-06-04 1:09 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Junio C Hamano
2 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
Move the body of git_config_parse_key() into a static helper
do_parse_config_key() that takes a "quiet" flag and treats
store_key as optional. git_config_parse_key() becomes a thin
wrapper.
Add git_config_key_is_valid() for callers that only need to
know whether a key is well-formed.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
config.c | 38 +++++++++++++++++++++++++++++---------
config.h | 2 ++
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/config.c b/config.c
index a1b92fe083..45144f73c5 100644
--- a/config.c
+++ b/config.c
@@ -536,11 +536,14 @@ static inline int iskeychar(int c)
* -2 if there is no section name in the key.
*
* store_key - pointer to char* which will hold a copy of the key with
- * lowercase section and variable name
+ * lowercase section and variable name, can be NULL to skip
+ * allocation when only validation is needed
* baselen - pointer to size_t which will hold the length of the
* section + subsection part, can be NULL
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+static int do_parse_config_key(const char *key, char **store_key,
+ size_t *baselen_, int quiet)
{
size_t i, baselen;
int dot;
@@ -552,12 +555,14 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(_("key does not contain a section: %s"), key);
+ if (!quiet)
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(_("key does not contain variable name: %s"), key);
+ if (!quiet)
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -568,7 +573,8 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
/*
* Validate the key and while at it, lower case it for matching.
*/
- *store_key = xmallocz(strlen(key));
+ if (store_key)
+ *store_key = xmallocz(strlen(key));
dot = 0;
for (i = 0; key[i]; i++) {
@@ -579,24 +585,38 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!dot || i > baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
- error(_("invalid key: %s"), key);
+ if (!quiet)
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(_("invalid key (newline): %s"), key);
+ if (!quiet)
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
- (*store_key)[i] = c;
+ if (store_key)
+ (*store_key)[i] = c;
}
return 0;
out_free_ret_1:
- FREE_AND_NULL(*store_key);
+ if (store_key)
+ FREE_AND_NULL(*store_key);
return -CONFIG_INVALID_KEY;
}
+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+{
+ return do_parse_config_key(key, store_key, baselen_, 0);
+}
+
+int git_config_key_is_valid(const char *key)
+{
+ return !do_parse_config_key(key, NULL, NULL, 1);
+}
+
static int config_parse_pair(const char *key, const char *value,
struct key_value_info *kvi,
config_fn_t fn, void *data)
diff --git a/config.h b/config.h
index bf47fb3afc..31fe3e2961 100644
--- a/config.h
+++ b/config.h
@@ -343,6 +343,8 @@ void repo_config_set(struct repository *, const char *, const char *);
int git_config_parse_key(const char *, char **, size_t *);
+int git_config_key_is_valid(const char *);
+
/*
* The following macros specify flag bits that alter the behavior
* of the repo_config_set_multivar*() methods.
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v6 2/2] config: improve diagnostic for "set" with missing value
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation Harald Nordgren via GitGitGadget
@ 2026-06-02 18:43 ` Harald Nordgren via GitGitGadget
2026-06-04 1:09 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Junio C Hamano
2 siblings, 0 replies; 36+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail•com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail•com>
---
builtin/config.c | 32 ++++++++++++++++++++++++++-
t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..8d8ec0beea 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,26 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && git_config_key_is_valid(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (git_config_key_is_valid(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1154,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1394,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1409,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1419,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..87ca11a127 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,62 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: digit-led section name is valid' '
+ test_must_fail git config set 1foo.bar=baz 2>err &&
+ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
+ test_grep "did you mean .git config set 1foo\\.bar baz." err
+'
+
+test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
+ test_must_fail git config set foo.some.b_r=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v5 1/2] config: let git_config_parse_key() validate quietly
2026-06-02 16:31 ` Harald Nordgren
@ 2026-06-04 1:09 ` Junio C Hamano
0 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-06-04 1:09 UTC (permalink / raw)
To: Harald Nordgren
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk
Harald Nordgren <haraldnordgren@gmail•com> writes:
>> Perhaps the updated "git_config_parse_key()" in this patch should be
>> renamed to be a file-scape static internal helper, and the existing
>> "git_config_parse_key()" should become a thin wrapper around that
>> new helper function, retaining the current external interface,
>> requiring no changes to existing callers.
>
> I want to remember a discussion on one of my earlier topics, a few
> months back, where someone else suggested instead of introducing two
> thin wrappers over a helper, we should update the callers instead.
>
> But for me either way is fine, maybe here it makes more sense, because
> of the repeated NULL/0/1 parameters.
If the "quiet" and "store_key" setting were independent, then I
wouldn't have made such a suggestion. But I got an impression that
with the updated code, there wasn't a valid use case to ask to
quietly store the discovered key.
An ideal refactoring would have been a low level helper function
that only yields error code, and git_config_parse_key() would call
it and react to the returned error code, stores the discovered key,
and produces error message on its own. Then such an "always quiet"
helper can be used for the purpose of the new caller, without having
to have "if (!quiet)" sprinkled all over. But that is certainly
cumbersome to arrange.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 0/2] config: suggest the correct form when key contains "="
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
@ 2026-06-04 1:09 ` Junio C Hamano
2 siblings, 0 replies; 36+ messages in thread
From: Junio C Hamano @ 2026-06-04 1:09 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Harald Nordgren
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail•com> writes:
> * The quiet parameter now lives on a static do_parse_config_key() instead
> of git_config_parse_key() itself. git_config_parse_key() is back to its
> three-argument signature; existing callers don't change.
> * New public git_config_key_is_valid() for callers that only need a yes/no
> check.
>
> Harald Nordgren (2):
> config: add git_config_key_is_valid() for quiet validation
> config: improve diagnostic for "set" with missing value
>
> builtin/config.c | 32 ++++++++++++++++++++++++++-
> config.c | 38 ++++++++++++++++++++++++--------
> config.h | 2 ++
> t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 118 insertions(+), 10 deletions(-)
Looking good. Thanks. Will queue.
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2026-06-04 1:09 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 13:58 [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-05-14 21:26 ` Junio C Hamano
2026-05-14 22:16 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-15 1:28 ` Junio C Hamano
2026-05-15 7:56 ` Email issues Harald Nordgren
2026-05-15 12:02 ` Kristoffer Haugsbakk
2026-05-15 9:39 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-16 12:51 ` [PATCH] config: suggest the correct form when key contains "=" Harald Nordgren
2026-05-16 12:52 ` [PATCH v2] config: suggest the correct form when key contains "=" in set context Harald Nordgren via GitGitGadget
2026-05-25 8:33 ` [PATCH v3] " Harald Nordgren via GitGitGadget
2026-05-25 9:15 ` Junio C Hamano
2026-05-26 19:21 ` [PATCH v4] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-05-26 19:24 ` Harald Nordgren
2026-06-01 23:45 ` Junio C Hamano
2026-06-01 23:53 ` Junio C Hamano
2026-06-02 13:39 ` [PATCH v5 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 13:39 ` [PATCH v5 1/2] config: let git_config_parse_key() validate quietly Harald Nordgren via GitGitGadget
2026-06-02 14:08 ` Junio C Hamano
2026-06-02 16:31 ` Harald Nordgren
2026-06-04 1:09 ` Junio C Hamano
2026-06-02 13:39 ` [PATCH v5 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-06-02 14:18 ` Junio C Hamano
2026-06-02 18:43 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation Harald Nordgren via GitGitGadget
2026-06-02 18:43 ` [PATCH v6 2/2] config: improve diagnostic for "set" with missing value Harald Nordgren via GitGitGadget
2026-06-04 1:09 ` [PATCH v6 0/2] config: suggest the correct form when key contains "=" Junio C Hamano
-- strict thread matches above, loose matches on Subject: below --
2026-05-13 13:46 [PATCH v8 0/5] branch: prune-merged Junio C Hamano
2026-05-13 18:57 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-12 13:53 [PATCH v7 3/5] branch: add --prune-merged <remote> Junio C Hamano
2026-05-12 17:00 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-11 23:20 [PATCH v6 0/5] branch: prune-merged Junio C Hamano
2026-05-12 7:35 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-11 8:18 [PATCH v5 2/5] branch: let delete_branches warn instead of error on bulk refusal Junio C Hamano
2026-05-11 8:44 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-05 20:48 [PATCH v4 4/6] fetch: add --prune-merged Johannes Sixt
2026-05-05 22:07 ` [PATCH] fetch: add fetch.pruneLocalBranches config Harald Nordgren
2026-05-11 2:59 ` Junio C Hamano
2026-05-11 6:56 ` Harald Nordgren
2026-05-01 21:35 Harald Nordgren via GitGitGadget
2026-05-03 22:39 ` Junio C Hamano
2026-05-05 7:14 ` Johannes Sixt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox