[2/9] submodule--helper: prefer strip_suffix() to ends_with()

Commit Message

Jeff King Nov. 12, 2018, 2:47 p.m. UTC

Using strip_suffix() lets us avoid repeating ourselves. It also makes
the handling of "/" a bit less subtle (we strip one less character than
we matched in order to leave it in place, but we can just as easily
include the "/" when we add more path components).

Signed-off-by: Jeff King <[e-mail]>
---
 builtin/submodule--helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Stefan Beller Nov. 12, 2018, 6:23 p.m. UTC | #1

On Mon, Nov 12, 2018 at 6:47 AM Jeff King <[e-mail]> wrote:
>
> Using strip_suffix() lets us avoid repeating ourselves. It also makes
> the handling of "/" a bit less subtle (we strip one less character than
> we matched in order to leave it in place, but we can just as easily
> include the "/" when we add more path components).
>
> Signed-off-by: Jeff King <[e-mail]>

This makes sense. Thanks!

(This patch caught my attention as it's a submodule thing,
but now looking at the rest of the series)

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 676175b9be..28b9449e82 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1268,16 +1268,17 @@  static int add_possible_reference_from_superproject(
 		struct alternate_object_database *alt, void *sas_cb)
 {
 	struct submodule_alternate_setup *sas = sas_cb;
+	size_t len;
 
 	/*
 	 * If the alternate object store is another repository, try the
 	 * standard layout with .git/(modules/<name>)+/objects
 	 */
-	if (ends_with(alt->path, "/objects")) {
+	if (strip_suffix(alt->path, "/objects", &len)) {
 		char *sm_alternate;
 		struct strbuf sb = STRBUF_INIT;
 		struct strbuf err = STRBUF_INIT;
-		strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects"));
+		strbuf_add(&sb, alt->path, len);
 
 		/*
 		 * We need to end the new path with '/' to mark it as a dir,
@@ -1285,7 +1286,7 @@  static int add_possible_reference_from_superproject(
 		 * as the last part of a missing submodule reference would
 		 * be taken as a file name.
 		 */
-		strbuf_addf(&sb, "modules/%s/", sas->submodule_name);
+		strbuf_addf(&sb, "/modules/%s/", sas->submodule_name);
 
 		sm_alternate = compute_alternate_path(sb.buf, &err);
 		if (sm_alternate) {