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

From: Jeff King <peff@peff.net>
Date: Dec 2, 2024, 20:51 UTC
Message-ID: <20241202205114.GC776185@coredump.intra.peff.net>
In-Reply-To: <20241202070714.3028549-3-gitster@pobox.com>

On Mon, Dec 02, 2024 at 04:07:12PM +0900, Junio C Hamano wrote:

Show 5 quoted lines

> 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