diff mbox series

[2/4] refs: drop strbuf_ prefix from helpers

Message ID 20241202070714.3028549-3-gitster@pobox.com (mailing list archive)
State Superseded
Headers show
Series forbid HEAD as a tagname | expand

Commit Message

Junio C Hamano Dec. 2, 2024, 7:07 a.m. UTC 
The helper functions (strbuf_branchname, strbuf_check_branch_ref,
and strbuf_check_tag_ref) are about handling branch and tag names,
and it is a non-essential fact that these functions use strbuf to
hold these names.  Rename them to make it clarify that these are
more about "ref".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 branch.c                   |  2 +-
 builtin/branch.c           | 10 +++++-----
 builtin/check-ref-format.c |  2 +-
 builtin/checkout.c         |  2 +-
 builtin/merge.c            |  2 +-
 builtin/tag.c              |  2 +-
 builtin/worktree.c         |  8 ++++----
 gitweb/gitweb.perl         |  2 +-
 refs.c                     |  8 ++++----
 refs.h                     |  8 ++++----
 10 files changed, 23 insertions(+), 23 deletions(-)

Comments

Jeff King Dec. 2, 2024, 8:51 p.m. UTC | #1 
On Mon, Dec 02, 2024 at 04:07:12PM +0900, Junio C Hamano wrote:

> The helper functions (strbuf_branchname, strbuf_check_branch_ref,
> and strbuf_check_tag_ref) are about handling branch and tag names,
> and it is a non-essential fact that these functions use strbuf to
> hold these names.  Rename them to make it clarify that these are
> more about "ref".

Sounds good. I wasn't quite sure about the name copy_branchname(), since
it actually expands/interprets the name. But the word "interpret" is
already used for another similar function, repo_interpret_branch_name().

In fact, this function is a very thin wrapper around it, which made me
wonder if it has any value. It looks like the main useful bit is that on
error it will copy the name verbatim.

So I guess it is really more like copy_or_expand_branchname(). I don't
know if that is really adding much, though. Probably just the name
copy_branchname(), coupled with the documentation above the declaration,
will be sufficient.

  As a side note, repo_interpret_branch_name() is in object-file.[ch],
  but probably should also be in refs.[ch], as in your first patch.
  Let's not worry about it for your series, though.

-Peff
diff mbox series

Patch

diff --git a/branch.c b/branch.c
index 08fa4094d2..58b61831af 100644
--- a/branch.c
+++ b/branch.c
@@ -372,7 +372,7 @@  int read_branch_desc(struct strbuf *buf, const char *branch_name)
  */
 int validate_branchname(const char *name, struct strbuf *ref)
 {
-	if (strbuf_check_branch_ref(ref, name)) {
+	if (check_branch_ref(ref, name)) {
 		int code = die_message(_("'%s' is not a valid branch name"), name);
 		advise_if_enabled(ADVICE_REF_SYNTAX,
 				  _("See `man git check-ref-format`"));
diff --git a/builtin/branch.c b/builtin/branch.c
index fd1611ebf5..17acf598d2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -257,7 +257,7 @@  static int delete_branches(int argc, const char **argv, int force, int kinds,
 		char *target = NULL;
 		int flags = 0;
 
-		strbuf_branchname(&bname, argv[i], allowed_interpret);
+		copy_branchname(&bname, argv[i], allowed_interpret);
 		free(name);
 		name = mkpathdup(fmt, bname.buf);
 
@@ -579,7 +579,7 @@  static void copy_or_rename_branch(const char *oldname, const char *newname, int
 	int recovery = 0, oldref_usage = 0;
 	struct worktree **worktrees = get_worktrees();
 
-	if (strbuf_check_branch_ref(&oldref, oldname)) {
+	if (check_branch_ref(&oldref, oldname)) {
 		/*
 		 * Bad name --- this could be an attempt to rename a
 		 * ref that we used to allow to be created by accident.
@@ -894,7 +894,7 @@  int cmd_branch(int argc,
 				die(_("cannot give description to detached HEAD"));
 			branch_name = head;
 		} else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch_name = buf.buf;
 		} else {
 			die(_("cannot edit description of more than one branch"));
@@ -933,7 +933,7 @@  int cmd_branch(int argc,
 		if (!argc)
 			branch = branch_get(NULL);
 		else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch = branch_get(buf.buf);
 		} else
 			die(_("too many arguments to set new upstream"));
@@ -963,7 +963,7 @@  int cmd_branch(int argc,
 		if (!argc)
 			branch = branch_get(NULL);
 		else if (argc == 1) {
-			strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+			copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
 			branch = branch_get(buf.buf);
 		} else
 			die(_("too many arguments to unset upstream"));
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index e86d8ef980..cef1ffe3ce 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -42,7 +42,7 @@  static int check_ref_format_branch(const char *arg)
 	int nongit;
 
 	setup_git_directory_gently(&nongit);
-	if (strbuf_check_branch_ref(&sb, arg) ||
+	if (check_branch_ref(&sb, arg) ||
 	    !skip_prefix(sb.buf, "refs/heads/", &name))
 		die("'%s' is not a valid branch name", arg);
 	printf("%s\n", name);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c449558e66..5e5afa0f26 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -742,7 +742,7 @@  static void setup_branch_path(struct branch_info *branch)
 			   &branch->oid, &branch->refname, 0))
 		repo_get_oid_committish(the_repository, branch->name, &branch->oid);
 
-	strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
+	copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
 	if (strcmp(buf.buf, branch->name)) {
 		free(branch->name);
 		branch->name = xstrdup(buf.buf);
diff --git a/builtin/merge.c b/builtin/merge.c
index 84d0f3604b..d0c31d7714 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -498,7 +498,7 @@  static void merge_name(const char *remote, struct strbuf *msg)
 	char *found_ref = NULL;
 	int len, early;
 
-	strbuf_branchname(&bname, remote, 0);
+	copy_branchname(&bname, remote, 0);
 	remote = bname.buf;
 
 	oidclr(&branch_head, the_repository->hash_algo);
diff --git a/builtin/tag.c b/builtin/tag.c
index 8279dccbe0..670e564178 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -639,7 +639,7 @@  int cmd_tag(int argc,
 	if (repo_get_oid(the_repository, object_ref, &object))
 		die(_("Failed to resolve '%s' as a valid ref."), object_ref);
 
-	if (strbuf_check_tag_ref(&ref, tag))
+	if (check_tag_ref(&ref, tag))
 		die(_("'%s' is not a valid tag name."), tag);
 
 	if (refs_read_ref(get_main_ref_store(the_repository), ref.buf, &prev))
diff --git a/builtin/worktree.c b/builtin/worktree.c
index fc31d072a6..c68f601358 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -432,7 +432,7 @@  static int add_worktree(const char *path, const char *refname,
 	worktrees = NULL;
 
 	/* is 'refname' a branch or commit? */
-	if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
+	if (!opts->detach && !check_branch_ref(&symref, refname) &&
 	    refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
 		is_branch = 1;
 		if (!opts->force)
@@ -604,7 +604,7 @@  static void print_preparing_worktree_line(int detach,
 		fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
 	} else {
 		struct strbuf s = STRBUF_INIT;
-		if (!detach && !strbuf_check_branch_ref(&s, branch) &&
+		if (!detach && !check_branch_ref(&s, branch) &&
 		    refs_ref_exists(get_main_ref_store(the_repository), s.buf))
 			fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
 				  branch);
@@ -745,7 +745,7 @@  static char *dwim_branch(const char *path, char **new_branch)
 	char *branchname = xstrndup(s, n);
 	struct strbuf ref = STRBUF_INIT;
 
-	branch_exists = !strbuf_check_branch_ref(&ref, branchname) &&
+	branch_exists = !check_branch_ref(&ref, branchname) &&
 			refs_ref_exists(get_main_ref_store(the_repository),
 					ref.buf);
 	strbuf_release(&ref);
@@ -838,7 +838,7 @@  static int add(int ac, const char **av, const char *prefix)
 		new_branch = new_branch_force;
 
 		if (!opts.force &&
-		    !strbuf_check_branch_ref(&symref, new_branch) &&
+		    !check_branch_ref(&symref, new_branch) &&
 		    refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
 			die_if_checked_out(symref.buf, 0);
 		strbuf_release(&symref);
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b09a8d0523..8cdb0d9b9f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2094,7 +2094,7 @@  sub format_log_line_html {
         (
             # The output of "git describe", e.g. v2.10.0-297-gf6727b0
             # or hadoop-20160921-113441-20-g094fb7d
-            (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
+            (?<!-) # see check_tag_ref(). Tags can't start with -
             [A-Za-z0-9.-]+
             (?!\.) # refs can't end with ".", see check_refname_format()
             -g$regex
diff --git a/refs.c b/refs.c
index 59a9223d4c..a24bfe3845 100644
--- a/refs.c
+++ b/refs.c
@@ -697,7 +697,7 @@  static char *substitute_branch_name(struct repository *r,
 	return NULL;
 }
 
-void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
+void copy_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 {
 	int len = strlen(name);
 	struct interpret_branch_name_options options = {
@@ -711,10 +711,10 @@  void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 	strbuf_add(sb, name + used, len - used);
 }
 
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
+int check_branch_ref(struct strbuf *sb, const char *name)
 {
 	if (startup_info->have_repository)
-		strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+		copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
 	else
 		strbuf_addstr(sb, name);
 
@@ -733,7 +733,7 @@  int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 	return check_refname_format(sb->buf, 0);
 }
 
-int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
+int check_tag_ref(struct strbuf *sb, const char *name)
 {
 	if (name[0] == '-')
 		return -1;
diff --git a/refs.h b/refs.h
index f19b0ad92f..c5280477f0 100644
--- a/refs.h
+++ b/refs.h
@@ -191,23 +191,23 @@  char *repo_default_branch_name(struct repository *r, int quiet);
  * If "allowed" is non-zero, restrict the set of allowed expansions. See
  * repo_interpret_branch_name() for details.
  */
-void strbuf_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct strbuf *sb, const char *name,
 		       unsigned allowed);
 
 /*
- * Like strbuf_branchname() above, but confirm that the result is
+ * Like copy_branchname() above, but confirm that the result is
  * syntactically valid to be used as a local branch name in refs/heads/.
  *
  * The return value is "0" if the result is valid, and "-1" otherwise.
  */
-int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
+int check_branch_ref(struct strbuf *sb, const char *name);
 
 /*
  * Similar for a tag name in refs/tags/.
  *
  * The return value is "0" if the result is valid, and "-1" otherwise.
  */
-int strbuf_check_tag_ref(struct strbuf *sb, const char *name);
+int check_tag_ref(struct strbuf *sb, const char *name);
 
 /*
  * A ref_transaction represents a collection of reference updates that