branch: split validate_new_branchname() into two
Checking if a proposed name is appropriate for a branch is strictly a subset of checking if we want to allow creating or updating a branch with such a name. The mysterious sounding 'attr_only' parameter to validate_new_branchname() is used to switch the function between these two roles. Instead, split the function into two, and adjust the callers. A new helper validate_branchname() only checks the name and reports if the branch already exists. This loses one NEEDSWORK from the branch API. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Oct 13, 2017 at 13:45 UTC
bc1c9c0e674bdd293c29ae84365915848ed01d7a
4 files changed
+44
-35
branch.c
+23
-11
@@ -178,19 +178,31 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
178
return 0;
179
}
180
181
-int validate_new_branchname(const char *name, struct strbuf *ref,
182
- int force, int attr_only)
181
+/*
182
+ * Check if 'name' can be a valid name for a branch; die otherwise.
183
+ * Return 1 if the named branch already exists; return 0 otherwise.
184
+ * Fill ref with the full refname for the branch.
185
+ */
186
+int validate_branchname(const char *name, struct strbuf *ref)
187
{
184
- const char *head;
185
-
188
if (strbuf_check_branch_ref(ref, name))
189
die(_("'%s' is not a valid branch name."), name);
190
189
- if (!ref_exists(ref->buf))
190
- return 0;
191
+ return ref_exists(ref->buf);
192
+}
193
192
- if (attr_only)
193
- return 1;
194
+/*
195
+ * Check if a branch 'name' can be created as a new branch; die otherwise.
196
+ * 'force' can be used when it is OK for the named branch already exists.
197
+ * Return 1 if the named branch already exists; return 0 otherwise.
198
+ * Fill ref with the full refname for the branch.
199
+ */
200
+int validate_new_branchname(const char *name, struct strbuf *ref, int force)
201
+{
202
+ const char *head;
203
+
204
+ if (!validate_branchname(name, ref))
205
+ return 0;
206
207
if (!force)
208
die(_("A branch named '%s' already exists."),
@@ -246,9 +258,9 @@ void create_branch(const char *name, const char *start_name,
258
if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
259
explicit_tracking = 1;
260
249
- if (validate_new_branchname(name, &ref, force,
250
- track == BRANCH_TRACK_OVERRIDE ||
251
- clobber_head)) {
261
+ if ((track == BRANCH_TRACK_OVERRIDE || clobber_head)
262
+ ? validate_branchname(name, &ref)
263
+ : validate_new_branchname(name, &ref, force)) {
264
if (!force)
265
dont_change_ref = 1;
266
else
branch.h
+12
-15
@@ -23,22 +23,19 @@ void create_branch(const char *name, const char *start_name,
23
int clobber_head, int quiet, enum branch_track track);
24
25
/*
26
- * Validates that the requested branch may be created, returning the
27
- * interpreted ref in ref, force indicates whether (non-head) branches
28
- * may be overwritten. A non-zero return value indicates that the force
29
- * parameter was non-zero and the branch already exists.
30
- *
31
- * Contrary to all of the above, when attr_only is 1, the caller is
32
- * not interested in verifying if it is Ok to update the named
33
- * branch to point at a potentially different commit. It is merely
34
- * asking if it is OK to change some attribute for the named branch
35
- * (e.g. tracking upstream).
36
- *
37
- * NEEDSWORK: This needs to be split into two separate functions in the
38
- * longer run for sanity.
39
- *
26
+ * Check if 'name' can be a valid name for a branch; die otherwise.
27
+ * Return 1 if the named branch already exists; return 0 otherwise.
28
+ * Fill ref with the full refname for the branch.
29
+ */
30
+extern int validate_branchname(const char *name, struct strbuf *ref);
31
+
32
+/*
33
+ * Check if a branch 'name' can be created as a new branch; die otherwise.
34
+ * 'force' can be used when it is OK for the named branch already exists.
35
+ * Return 1 if the named branch already exists; return 0 otherwise.
36
+ * Fill ref with the full refname for the branch.
37
*/
41
-int validate_new_branchname(const char *name, struct strbuf *ref, int force, int attr_only);
38
+extern int validate_new_branchname(const char *name, struct strbuf *ref, int force);
39
40
/*
41
* Remove information about the state of working on the current
builtin/branch.c
+4
-4
@@ -463,7 +463,6 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
463
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
464
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
465
int recovery = 0;
466
- int clobber_head_ok;
466
467
if (!oldname) {
468
if (copy)
@@ -487,9 +486,10 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
486
* A command like "git branch -M currentbranch currentbranch" cannot
487
* cause the worktree to become inconsistent with HEAD, so allow it.
488
*/
490
- clobber_head_ok = !strcmp(oldname, newname);
491
-
492
- validate_new_branchname(newname, &newref, force, clobber_head_ok);
489
+ if (!strcmp(oldname, newname))
490
+ validate_branchname(newname, &newref);
491
+ else
492
+ validate_new_branchname(newname, &newref, force);
493
494
reject_rebase_or_bisect_branch(oldref.buf);
495
builtin/checkout.c
+5
-5
@@ -1289,11 +1289,11 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1289
if (opts.new_branch) {
1290
struct strbuf buf = STRBUF_INIT;
1291
1292
- opts.branch_exists =
1293
- validate_new_branchname(opts.new_branch, &buf,
1294
- !!opts.new_branch_force,
1295
- !!opts.new_branch_force);
1296
-
1292
+ if (opts.new_branch_force)
1293
+ opts.branch_exists = validate_branchname(opts.new_branch, &buf);
1294
+ else
1295
+ opts.branch_exists =
1296
+ validate_new_branchname(opts.new_branch, &buf, 0);
1297
strbuf_release(&buf);
1298
}
1299