{"thread":{"id":"58420","subject":"[PATCH] refs: unify parse_worktree_ref() and ref_type()","startedAt":"2022-09-12T17:01:46Z","lastAt":"2022-09-21T16:50:43Z","messageCount":9,"participants":["Han-Wen Nienhuys via GitGitGadget","Junio C Hamano","Han-Wen Nienhuys"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"462940","messageId":"pull.1325.git.git.1663002096207.gitgitgadget@gmail.com","threadId":"58420","inReplyTo":null,"subject":"[PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-12T17:01:35Z","receivedAt":"2022-09-12T17:01:46Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys <hanwen@google.com>\n\nThe logic to handle worktree refs (worktrees/NAME/REF and\nmain-worktree/REF) existed in two places:\n\n* ref_type() in refs.c\n\n* parse_worktree_ref() in worktree.c\n\nCollapse this logic together in one function parse_worktree_ref():\nthis avoids having to cross-check the result of parse_worktree_ref()\nand ref_type().\n\nIntroduce enum ref_worktree_type, which is slightly different from\nenum ref_type. The latter is a misleading name (one would think that\n'ref_type' would have the symref option).\n\nInstead, enum ref_worktree_type only makes explicit how a refname\nrelates to a worktree. From this point of view, HEAD and\nrefs/bisect/abc are the same: they specify the current worktree\nimplicitly.\n\nThe files-backend must avoid packing refs/bisect/* and friends into\npacked-refs, so expose is_per_worktree_ref() separately.\n\nSigned-off-by: Han-Wen Nienhuys <hanwen@google.com>\n---\n    refs: unify parse_worktree_ref() and ref_type()\n    \n    The logic to handle worktree refs (worktrees/NAME/REF and\n    main-worktree/REF) existed in two places:\n    \n     * ref_type() in refs.c\n    \n     * parse_worktree_ref() in worktree.c\n    \n    Collapse this logic together in one function parse_worktree_ref(): this\n    avoids having to cross-check the result of parse_worktree_ref() and\n    ref_type().\n    \n    Introduce enum ref_worktree_type, which is slightly different from enum\n    ref_type. The latter is a misleading name (one would think that\n    'ref_type' would have the symref option).\n    \n    Instead, enum ref_worktree_type only makes explicit how a refname\n    relates to a worktree. From this point of view, HEAD and refs/bisect/abc\n    are the same: they specify the current worktree implicitly.\n    \n    The files-backend must avoid packing refs/bisect/* and friends into\n    packed-refs, so expose is_per_worktree_ref() separately.\n    \n    Signed-off-by: Han-Wen Nienhuys hanwen@google.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1325%2Fhanwen%2Fparse-worktree-ref-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1325/hanwen/parse-worktree-ref-v1\nPull-Request: https://github.com/git/git/pull/1325\n\n builtin/reflog.c      |  3 +-\n reflog.c              | 13 ++-----\n refs.c                | 75 ++++++++++++++++++++++++++--------------\n refs.h                | 29 ++++++++++++----\n refs/files-backend.c  | 80 +++++++++++++++++++------------------------\n refs/packed-backend.c |  2 +-\n worktree.c            | 59 ++++---------------------------\n worktree.h            | 10 ------\n 8 files changed, 120 insertions(+), 151 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 9407f835cb6..bd568d2d931 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -26,7 +26,8 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus\n \t * Avoid collecting the same shared ref multiple times because\n \t * they are available via all worktrees.\n \t */\n-\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n+\tif (!worktree->is_current &&\n+\t    parse_worktree_ref(ref, NULL, NULL, NULL) == REF_WORKTREE_SHARED)\n \t\treturn 0;\n \n \tstrbuf_worktree_ref(worktree, &newref, ref);\ndiff --git a/reflog.c b/reflog.c\nindex 47ba8620c56..0b8b767f97c 100644\n--- a/reflog.c\n+++ b/reflog.c\n@@ -310,16 +310,9 @@ static int push_tip_to_list(const char *refname, const struct object_id *oid,\n \n static int is_head(const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tif (parse_worktree_ref(refname, NULL, NULL, &refname))\n-\t\t\tBUG(\"not a worktree ref: %s\", refname);\n-\t\tbreak;\n-\tdefault:\n-\t\tbreak;\n-\t}\n-\treturn !strcmp(refname, \"HEAD\");\n+\tconst char *stripped_refname;\n+\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n+\treturn !strcmp(stripped_refname, \"HEAD\");\n }\n \n void reflog_expiry_prepare(const char *refname,\ndiff --git a/refs.c b/refs.c\nindex 1a964505f92..45d59a7ede2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -722,7 +722,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \treturn repo_dwim_log(the_repository, str, len, oid, log);\n }\n \n-static int is_per_worktree_ref(const char *refname)\n+int is_per_worktree_ref(const char *refname)\n {\n \treturn starts_with(refname, \"refs/worktree/\") ||\n \t       starts_with(refname, \"refs/bisect/\") ||\n@@ -738,37 +738,60 @@ static int is_pseudoref_syntax(const char *refname)\n \t\t\treturn 0;\n \t}\n \n+\t/* HEAD is not a pseudoref, but it certainly uses the\n+\t * pseudoref syntax. */\n \treturn 1;\n }\n \n-static int is_main_pseudoref_syntax(const char *refname)\n+enum ref_worktree_type parse_worktree_ref(const char *worktree_ref,\n+\t\t\t\t\t  const char **name, int *name_length,\n+\t\t\t\t\t  const char **ref)\n {\n-\treturn skip_prefix(refname, \"main-worktree/\", &refname) &&\n-\t\t*refname &&\n-\t\tis_pseudoref_syntax(refname);\n-}\n+\tconst char *name_dummy;\n+\tint name_length_dummy;\n+\tconst char *ref_dummy;\n+\tif (!name)\n+\t\tname = &name_dummy;\n+\tif (!name_length)\n+\t\tname_length = &name_length_dummy;\n+\tif (!ref)\n+\t\tref = &ref_dummy;\n \n-static int is_other_pseudoref_syntax(const char *refname)\n-{\n-\tif (!skip_prefix(refname, \"worktrees/\", &refname))\n-\t\treturn 0;\n-\trefname = strchr(refname, '/');\n-\tif (!refname || !refname[1])\n-\t\treturn 0;\n-\treturn is_pseudoref_syntax(refname + 1);\n-}\n+\t*ref = worktree_ref;\n+\tif (is_pseudoref_syntax(worktree_ref)) {\n+\t\treturn REF_WORKTREE_CURRENT;\n+\t}\n \n-enum ref_type ref_type(const char *refname)\n-{\n-\tif (is_per_worktree_ref(refname))\n-\t\treturn REF_TYPE_PER_WORKTREE;\n-\tif (is_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_PSEUDOREF;\n-\tif (is_main_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_MAIN_PSEUDOREF;\n-\tif (is_other_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_OTHER_PSEUDOREF;\n-\treturn REF_TYPE_NORMAL;\n+\tif (is_per_worktree_ref(worktree_ref)) {\n+\t\treturn REF_WORKTREE_CURRENT;\n+\t}\n+\n+\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n+\t\tif (!*worktree_ref)\n+\t\t\treturn -1;\n+\t\t*name = NULL;\n+\t\t*name_length = 0;\n+\t\t*ref = worktree_ref;\n+\n+\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n+\t\t    REF_WORKTREE_CURRENT)\n+\t\t\treturn REF_WORKTREE_MAIN;\n+\t}\n+\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n+\t\tconst char *slash = strchr(worktree_ref, '/');\n+\n+\t\tif (!slash || slash == worktree_ref || !slash[1])\n+\t\t\treturn -1;\n+\t\t*name = worktree_ref;\n+\t\t*name_length = slash - worktree_ref;\n+\t\t*ref = slash + 1;\n+\n+\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n+\t\t    REF_WORKTREE_CURRENT)\n+\t\t\treturn REF_WORKTREE_OTHER;\n+\t}\n+\n+\treturn REF_WORKTREE_SHARED;\n }\n \n long get_files_ref_lock_timeout_ms(void)\ndiff --git a/refs.h b/refs.h\nindex 23479c7ee09..9e40efc4787 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -825,15 +825,30 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);\n  */\n int ref_is_hidden(const char *, const char *);\n \n-enum ref_type {\n-\tREF_TYPE_PER_WORKTREE,\t  /* refs inside refs/ but not shared       */\n-\tREF_TYPE_PSEUDOREF,\t  /* refs outside refs/ in current worktree */\n-\tREF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */\n-\tREF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */\n-\tREF_TYPE_NORMAL,\t  /* normal/shared refs inside refs/        */\n+/* Is this a per-worktree ref living in the refs/ namespace? */\n+int is_per_worktree_ref(const char *refname);\n+\n+/* Describes how a refname relates to worktrees */\n+enum ref_worktree_type {\n+\tREF_WORKTREE_CURRENT, /* implicitly per worktree, eg. HEAD or\n+\t\t\t\t refs/bisect/SOMETHING */\n+\tREF_WORKTREE_MAIN, /* explicitly in main worktree, eg.\n+\t\t\t      refs/main-worktree/HEAD */\n+\tREF_WORKTREE_OTHER, /* explicitly in named worktree, eg.\n+\t\t\t       refs/worktrees/bla/HEAD */\n+\tREF_WORKTREE_SHARED, /* the default, eg. refs/heads/main */\n };\n \n-enum ref_type ref_type(const char *refname);\n+/* Parse a ref that possibly explicitly refers to a worktree ref\n+ * (ie. either REFNAME, main-worktree/REFNAME or\n+ * worktree/WORKTREE/REFNAME). If the name references a worktree\n+ * implicitly or explicitly, return what kind it was. The\n+ * worktree_name, worktree_name_length and refname argument maybe NULL.\n+ */\n+enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n+\t\t\t\t\t  const char **worktree_name,\n+\t\t\t\t\t  int *worktree_name_length,\n+\t\t\t\t\t  const char **refname);\n \n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 95acab78eef..f230704229e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -138,44 +138,30 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,\n \treturn refs;\n }\n \n-static void files_reflog_path_other_worktrees(struct files_ref_store *refs,\n-\t\t\t\t\t      struct strbuf *sb,\n-\t\t\t\t\t      const char *refname)\n-{\n-\tconst char *real_ref;\n-\tconst char *worktree_name;\n-\tint length;\n-\n-\tif (parse_worktree_ref(refname, &worktree_name, &length, &real_ref))\n-\t\tBUG(\"refname %s is not a other-worktree ref\", refname);\n-\n-\tif (worktree_name)\n-\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/logs/%s\", refs->gitcommondir,\n-\t\t\t    length, worktree_name, real_ref);\n-\telse\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir,\n-\t\t\t    real_ref);\n-}\n-\n static void files_reflog_path(struct files_ref_store *refs,\n \t\t\t      struct strbuf *sb,\n \t\t\t      const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PER_WORKTREE:\n-\tcase REF_TYPE_PSEUDOREF:\n+\tconst char *bare_refname;\n+\tconst char *wtname;\n+\tint wtname_len;\n+\tenum ref_worktree_type wt_type = parse_worktree_ref(\n+\t\trefname, &wtname, &wtname_len, &bare_refname);\n+\n+\tswitch (wt_type) {\n+\tcase REF_WORKTREE_CURRENT:\n \t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->base.gitdir, refname);\n \t\tbreak;\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tfiles_reflog_path_other_worktrees(refs, sb, refname);\n+\tcase REF_WORKTREE_SHARED:\n+\tcase REF_WORKTREE_MAIN:\n+\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir, bare_refname);\n \t\tbreak;\n-\tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir, refname);\n+\tcase REF_WORKTREE_OTHER:\n+\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/logs/%s\", refs->gitcommondir,\n+\t\t\t    wtname_len, wtname, bare_refname);\n \t\tbreak;\n \tdefault:\n-\t\tBUG(\"unknown ref type %d of ref %s\",\n-\t\t    ref_type(refname), refname);\n+\t\tBUG(\"unknown ref type %d of ref %s\", wt_type, refname);\n \t}\n }\n \n@@ -183,22 +169,25 @@ static void files_ref_path(struct files_ref_store *refs,\n \t\t\t   struct strbuf *sb,\n \t\t\t   const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PER_WORKTREE:\n-\tcase REF_TYPE_PSEUDOREF:\n+\tconst char *bare_refname;\n+\tconst char *wtname;\n+\tint wtname_len;\n+\tenum ref_worktree_type wt_type = parse_worktree_ref(\n+\t\trefname, &wtname, &wtname_len, &bare_refname);\n+\tswitch (wt_type) {\n+\tcase REF_WORKTREE_CURRENT:\n \t\tstrbuf_addf(sb, \"%s/%s\", refs->base.gitdir, refname);\n \t\tbreak;\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tif (!skip_prefix(refname, \"main-worktree/\", &refname))\n-\t\t\tBUG(\"ref %s is not a main pseudoref\", refname);\n-\t\t/* fallthrough */\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitcommondir, refname);\n+\tcase REF_WORKTREE_OTHER:\n+\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/%s\", refs->gitcommondir,\n+\t\t\t    wtname_len, wtname, bare_refname);\n+\t\tbreak;\n+\tcase REF_WORKTREE_SHARED:\n+\tcase REF_WORKTREE_MAIN:\n+\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitcommondir, bare_refname);\n \t\tbreak;\n \tdefault:\n-\t\tBUG(\"unknown ref type %d of ref %s\",\n-\t\t    ref_type(refname), refname);\n+\t\tBUG(\"unknown ref type %d of ref %s\", wt_type, refname);\n \t}\n }\n \n@@ -771,7 +760,8 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator)\n \n \twhile ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) {\n \t\tif (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&\n-\t\t    ref_type(iter->iter0->refname) != REF_TYPE_PER_WORKTREE)\n+\t\t    parse_worktree_ref(iter->iter0->refname, NULL, NULL,\n+\t\t\t\t       NULL) != REF_WORKTREE_CURRENT)\n \t\t\tcontinue;\n \n \t\tif ((iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS) &&\n@@ -1179,7 +1169,8 @@ static int should_pack_ref(const char *refname,\n \t\t\t   unsigned int pack_flags)\n {\n \t/* Do not pack per-worktree refs: */\n-\tif (ref_type(refname) != REF_TYPE_NORMAL)\n+\tif (parse_worktree_ref(refname, NULL, NULL, NULL) !=\n+\t    REF_WORKTREE_SHARED)\n \t\treturn 0;\n \n \t/* Do not pack non-tags unless PACK_REFS_ALL is set: */\n@@ -2277,7 +2268,8 @@ static enum iterator_selection reflog_iterator_select(\n \t\t */\n \t\treturn ITER_SELECT_0;\n \t} else if (iter_common) {\n-\t\tif (ref_type(iter_common->refname) == REF_TYPE_NORMAL)\n+\t\tif (parse_worktree_ref(iter_common->refname, NULL, NULL,\n+\t\t\t\t       NULL) == REF_WORKTREE_SHARED)\n \t\t\treturn ITER_SELECT_1;\n \n \t\t/*\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 66c4574c99d..bf0e63ae70c 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -862,7 +862,7 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)\n \n \twhile ((ok = next_record(iter)) == ITER_OK) {\n \t\tif (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&\n-\t\t    ref_type(iter->base.refname) != REF_TYPE_PER_WORKTREE)\n+\t\t    !is_per_worktree_ref(iter->base.refname))\n \t\t\tcontinue;\n \n \t\tif (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\ndiff --git a/worktree.c b/worktree.c\nindex 90fc085f76b..bb7873c72d1 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -489,62 +489,17 @@ int submodule_uses_worktrees(const char *path)\n \treturn ret;\n }\n \n-int parse_worktree_ref(const char *worktree_ref, const char **name,\n-\t\t       int *name_length, const char **ref)\n-{\n-\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n-\t\tif (!*worktree_ref)\n-\t\t\treturn -1;\n-\t\tif (name)\n-\t\t\t*name = NULL;\n-\t\tif (name_length)\n-\t\t\t*name_length = 0;\n-\t\tif (ref)\n-\t\t\t*ref = worktree_ref;\n-\t\treturn 0;\n-\t}\n-\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n-\t\tconst char *slash = strchr(worktree_ref, '/');\n-\n-\t\tif (!slash || slash == worktree_ref || !slash[1])\n-\t\t\treturn -1;\n-\t\tif (name)\n-\t\t\t*name = worktree_ref;\n-\t\tif (name_length)\n-\t\t\t*name_length = slash - worktree_ref;\n-\t\tif (ref)\n-\t\t\t*ref = slash + 1;\n-\t\treturn 0;\n-\t}\n-\treturn -1;\n-}\n-\n void strbuf_worktree_ref(const struct worktree *wt,\n \t\t\t struct strbuf *sb,\n \t\t\t const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PSEUDOREF:\n-\tcase REF_TYPE_PER_WORKTREE:\n-\t\tif (wt && !wt->is_current) {\n-\t\t\tif (is_main_worktree(wt))\n-\t\t\t\tstrbuf_addstr(sb, \"main-worktree/\");\n-\t\t\telse\n-\t\t\t\tstrbuf_addf(sb, \"worktrees/%s/\", wt->id);\n-\t\t}\n-\t\tbreak;\n-\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\t\tbreak;\n-\n-\tcase REF_TYPE_NORMAL:\n-\t\t/*\n-\t\t * For shared refs, don't prefix worktrees/ or\n-\t\t * main-worktree/. It's not necessary and\n-\t\t * files-backend.c can't handle it anyway.\n-\t\t */\n-\t\tbreak;\n+\tif (parse_worktree_ref(refname, NULL, NULL, NULL) ==\n+\t\t    REF_WORKTREE_CURRENT &&\n+\t    wt && !wt->is_current) {\n+\t\tif (is_main_worktree(wt))\n+\t\t\tstrbuf_addstr(sb, \"main-worktree/\");\n+\t\telse\n+\t\t\tstrbuf_addf(sb, \"worktrees/%s/\", wt->id);\n \t}\n \tstrbuf_addstr(sb, refname);\n }\ndiff --git a/worktree.h b/worktree.h\nindex e9e839926b0..9dcea6fc8c1 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -166,16 +166,6 @@ const char *worktree_git_path(const struct worktree *wt,\n \t\t\t      const char *fmt, ...)\n \t__attribute__((format (printf, 2, 3)));\n \n-/*\n- * Parse a worktree ref (i.e. with prefix main-worktree/ or\n- * worktrees/) and return the position of the worktree's name and\n- * length (or NULL and zero if it's main worktree), and ref.\n- *\n- * All name, name_length and ref arguments could be NULL.\n- */\n-int parse_worktree_ref(const char *worktree_ref, const char **name,\n-\t\t       int *name_length, const char **ref);\n-\n /*\n  * Return a refname suitable for access from the current ref store.\n  */\n\nbase-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n-- \ngitgitgadget\n"},{"id":"462945","messageId":"xmqq1qsge71x.fsf@gitster.g","threadId":"58420","inReplyTo":"pull.1325.git.git.1663002096207.gitgitgadget@gmail.com","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-12T19:17:30Z","receivedAt":"2022-09-12T19:17:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Han-Wen Nienhuys via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Han-Wen Nienhuys <hanwen@google.com>\n>\n> The logic to handle worktree refs (worktrees/NAME/REF and\n> main-worktree/REF) existed in two places:\n>\n> * ref_type() in refs.c\n>\n> * parse_worktree_ref() in worktree.c\n>\n> Collapse this logic together in one function parse_worktree_ref():\n> this avoids having to cross-check the result of parse_worktree_ref()\n> and ref_type().\n> ...\n> The files-backend must avoid packing refs/bisect/* and friends into\n> packed-refs, so expose is_per_worktree_ref() separately.\n\nA sensible goal.\n\n> diff --git a/builtin/reflog.c b/builtin/reflog.c\n> index 9407f835cb6..bd568d2d931 100644\n> --- a/builtin/reflog.c\n> +++ b/builtin/reflog.c\n> @@ -26,7 +26,8 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus\n>  \t * Avoid collecting the same shared ref multiple times because\n>  \t * they are available via all worktrees.\n>  \t */\n> -\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n> +\tif (!worktree->is_current &&\n> +\t    parse_worktree_ref(ref, NULL, NULL, NULL) == REF_WORKTREE_SHARED)\n>  \t\treturn 0;\n\nWe used to say \"for a ref without anything magical\" but now we say\n\"for a ref that is not per worktree at all\", and they mean the same\nthing.  OK.\n\n>  \tstrbuf_worktree_ref(worktree, &newref, ref);\n> diff --git a/reflog.c b/reflog.c\n> index 47ba8620c56..0b8b767f97c 100644\n> --- a/reflog.c\n> +++ b/reflog.c\n> @@ -310,16 +310,9 @@ static int push_tip_to_list(const char *refname, const struct object_id *oid,\n>  \n>  static int is_head(const char *refname)\n>  {\n> -\tswitch (ref_type(refname)) {\n> -\tcase REF_TYPE_OTHER_PSEUDOREF:\n> -\tcase REF_TYPE_MAIN_PSEUDOREF:\n> -\t\tif (parse_worktree_ref(refname, NULL, NULL, &refname))\n> -\t\t\tBUG(\"not a worktree ref: %s\", refname);\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tbreak;\n> -\t}\n> -\treturn !strcmp(refname, \"HEAD\");\n\nIt used to do some sort of input validation with BUG() but it no\nlonger does.  Intended?\n\nThe new call to parse_worktree_ref() is only to strip the possible\n\"main-worktree/\" and/or \"worktrees/$name/\" names.\n\n> +\tconst char *stripped_refname;\n> +\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n> +\treturn !strcmp(stripped_refname, \"HEAD\");\n>  }\n\n\n> @@ -738,37 +738,60 @@ static int is_pseudoref_syntax(const char *refname)\n>  \t\t\treturn 0;\n>  \t}\n>  \n> +\t/* HEAD is not a pseudoref, but it certainly uses the\n> +\t * pseudoref syntax. */\n\n\n/*\n * Our multi-line comments have opening slash-aster\n * and closing aster-slash on their own line.\n */\n\n\nNot your fault but the fault of the original file contents and diff\nalgorithms; the patch around here is simply unreadble because almost\nnothing in the preimage contributes to the result.  So I'll remove\n'-' lines first before commenting.\n\n> +enum ref_worktree_type parse_worktree_ref(const char *worktree_ref,\n> +\t\t\t\t\t  const char **name, int *name_length,\n> +\t\t\t\t\t  const char **ref)\n\nIt is hard to see what's input and what's output.  You have comments\nin <refs.h> but the prototype uses different parameter names and\nthey do not tell what is input and what is output, either.\n\n>  {\n> +\tconst char *name_dummy;\n> +\tint name_length_dummy;\n> +\tconst char *ref_dummy;\n\nStyle: Have blank here between the end of decls and the beginning of statements.\n\n> +\tif (!name)\n> +\t\tname = &name_dummy;\n> +\tif (!name_length)\n> +\t\tname_length = &name_length_dummy;\n> +\tif (!ref)\n> +\t\tref = &ref_dummy;\n\nI can guess from these that worktree_ref is the input and all three\nare to point at variables that receive optional output.  The patch\nauthor shouldn't make the code reader guess.\n\n> +\t*ref = worktree_ref;\n> +\tif (is_pseudoref_syntax(worktree_ref)) {\n> +\t\treturn REF_WORKTREE_CURRENT;\n> +\t}\n\nStyle: No braces around a single statement block.\n\nOK, given a worktree_ref, FETCH_HEAD, MERGE_HEAD, etc. are all valid\nonly for the current worktree.\n\n> +\tif (is_per_worktree_ref(worktree_ref)) {\n> +\t\treturn REF_WORKTREE_CURRENT;\n> +\t}\n\nAlso the ones that are within a certain known namespaces, like\n\"refs/bisect/*\".\n\nIn the above two cases, name/name_length are undefined.  Presumably\nthe caller behaves well enough not to peek into them unless it is\ntold to by the returned value REF_WORKTREE_OTHER?\n\n> +\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n\nThe special syntax \"main-worktree/$their_ref\".  We skip the prefix\nand then ...\n\n> +\t\tif (!*worktree_ref)\n> +\t\t\treturn -1;\n\nDo we know if, or do we have control over, \"enum ref_worktree_type\"\nends up being signed or unsigned?  I am not sure if the callers in\nthis patch even pays attention to the error condition.\n\n> +\t\t*name = NULL;\n> +\t\t*name_length = 0;\n> +\t\t*ref = worktree_ref;\n> +\n> +\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n> +\t\t    REF_WORKTREE_CURRENT)\n> +\t\t\treturn REF_WORKTREE_MAIN;\n\nIn \"main-worktree/$their_ref\", if $their_ref is the per worktree\nname, then it is a ref specific to the primary worktree.  \n\nIt is not clear what *name and *name_length are trying to return to\nthe caller here.\n\nOtherwise we fall through with worktree_ref that we have stripped\nmain-worktree/ prefix, which means the original input\n\n\tmain-worktree/worktrees/foo/blah\n\nis now \n\n\tworktrees/foo/blah\n\nand the next skip_prefix() will see that it begins with \"worktrees/\".\nOf course, if the initial input were\n\n\tworktrees/foo/blah\n\nthen we wouldn't have skipped main-worktree/ prefix from it, and go\nto the next skip_prefix().  So from here on, we cannot tell which\ncase the original input was.\n\nBut that is OK.  Asking \"give me the ref 'blah' in the worktree 'foo'\"\nin the current worktree should yield the same answer to the question\n\"give me the ref 'blah' in the worktree 'foo', as if I asked you to\ndo so in the main worktree\".\n\n> +\t}\n> +\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n> +\t\tconst char *slash = strchr(worktree_ref, '/');\n> +\n> +\t\tif (!slash || slash == worktree_ref || !slash[1])\n> +\t\t\treturn -1;\n> +\t\t*name = worktree_ref;\n> +\t\t*name_length = slash - worktree_ref;\n> +\t\t*ref = slash + 1;\n\nOK, name and name_length are giving the name of the worktree to the\ncaller here.  But this is done even when we do not end up returning\nREF_WORKTREE_OTHER, which sounds eh, ugly.\n\nIn any case, the part after worktrees/$worktreename/ is a per\nworktree name, then we say \"that's a per-worktree ref that belongs\nto $worktreename\" by returning _OTHER and with *name/*name_length.\n\n> +\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n> +\t\t    REF_WORKTREE_CURRENT)\n> +\t\t\treturn REF_WORKTREE_OTHER;\n\n> +\t}\n> +\n> +\treturn REF_WORKTREE_SHARED;\n\nAnd everything else is _SHARED.\n\nThe logic sounds sane enough, but the name/name_length being\nsometimes undefined and sometimes cleared feels yucky to me. \n\n>  }\n>  \n>  long get_files_ref_lock_timeout_ms(void)\n> diff --git a/refs.h b/refs.h\n> index 23479c7ee09..9e40efc4787 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -825,15 +825,30 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);\n>   */\n>  int ref_is_hidden(const char *, const char *);\n>  \n> +/* Is this a per-worktree ref living in the refs/ namespace? */\n> +int is_per_worktree_ref(const char *refname);\n> +\n> +/* Describes how a refname relates to worktrees */\n> +enum ref_worktree_type {\n> +\tREF_WORKTREE_CURRENT, /* implicitly per worktree, eg. HEAD or\n> +\t\t\t\t refs/bisect/SOMETHING */\n\nOK.\n\n> +\tREF_WORKTREE_MAIN, /* explicitly in main worktree, eg.\n> +\t\t\t      refs/main-worktree/HEAD */\n> +\tREF_WORKTREE_OTHER, /* explicitly in named worktree, eg.\n> +\t\t\t       refs/worktrees/bla/HEAD */\n\nHmph.  Do we need refs/ prefix before main-worktree/ and\nworktrees/bla?  The code we reviewed above seemed to skip\n\"main-worktree/\" and \"worktrees/\" with the assumption that there\nwill be no \"refs/\" prefix before them.  Probably just two typoes,\nas the comment before parse_worktree_ref() below also seems to\nassume that there is no refs/ before them.\n\n> +\tREF_WORKTREE_SHARED, /* the default, eg. refs/heads/main */\n>  };\n>  \n> -enum ref_type ref_type(const char *refname);\n> +/* Parse a ref that possibly explicitly refers to a worktree ref\n> + * (ie. either REFNAME, main-worktree/REFNAME or\n> + * worktree/WORKTREE/REFNAME). If the name references a worktree\n> + * implicitly or explicitly, return what kind it was. The\n> + * worktree_name, worktree_name_length and refname argument maybe NULL.\n> + */\n\nHere is my attempt to clarify what the comment tries to explain.\n\n\t/*\n\t * Parse maybe_worktree_ref (input) that possibly explicitly\n\t * refers to a worktree ref (i.e. REFNAME, main-worktree/REFNAME,\n\t * main-worktree/worktrees/WORKTREE/REFNAME, or\n\t * worktrees/WORKTREE/REFNAME).  Return what kind of ref\n         * (among the four kinds listed above) it is.\n         * \n\t * The location pointed at by worktree_name and worktree_name_length\n\t * are modified to point to \"WORKTREE\" part of such an input string\n         * when the returned value is REF_WORKTREE_OTHER.\n\t * Otherwise they are undefined (they may still be smudged).\n\t *\n\t * refname is made to piont to REFNAME part of the input string in\n\t * all cases.\n\t *\n\t * These three output parameters are optional and NULL can\n\t * be given for them.\n\t */\n\n> +enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n> +\t\t\t\t\t  const char **worktree_name,\n> +\t\t\t\t\t  int *worktree_name_length,\n> +\t\t\t\t\t  const char **refname);\n"},{"id":"462949","messageId":"xmqq35cwcpws.fsf@gitster.g","threadId":"58420","inReplyTo":"xmqq1qsge71x.fsf@gitster.g","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-12T20:13:07Z","receivedAt":"2022-09-12T20:13:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Otherwise we fall through with worktree_ref that we have stripped\n> main-worktree/ prefix, which means the original input\n>\n> \tmain-worktree/worktrees/foo/blah\n>\n> is now \n>\n> \tworktrees/foo/blah\n>\n> and the next skip_prefix() will see that it begins with \"worktrees/\".\n> Of course, if the initial input were\n>\n> \tworktrees/foo/blah\n>\n> then we wouldn't have skipped main-worktree/ prefix from it, and go\n> to the next skip_prefix().  So from here on, we cannot tell which\n> case the original input was.\n>\n> But that is OK.  Asking \"give me the ref 'blah' in the worktree 'foo'\"\n> in the current worktree should yield the same answer to the question\n> \"give me the ref 'blah' in the worktree 'foo', as if I asked you to\n> do so in the main worktree\".\n\nThis makes me wonder...\n\nI wonder if it makes the resulting code clearer to go fully\nrecursive, unlike the posted code that says \"if a recursive call\nsays it is for current, that means it is for main worktree, and\notherwise pretend as if the input did not have the prefix\".\n\nThat is, something like\n\nparse_worktree_ref(...) {\n\n\t... prepare name, name_len and ref ...\n\n\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n\t\tparsed = parse_worktree_ref(worktree_ref, name, name_len, ref);\n        \tswitch (parsed) {\n\t\tcase REF_WORKTREE_CURRENT:\n                case REF_WORKTREE_MAIN:\n\t\t\treturn REF_WORKTREE_MAIN;\n\t\tcase REF_WORKTREE_OTHER:\n\t\tcase REF_WORKTREE_SHARED:\n\t\t\treturn parsed;\n\t\t}\n\t}\n\n\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n\t\tslash = strchr(worktree_ref, '/');\n\n\t\tparsed = parse_worktree_ref(slash + 1, name, name_len, ref);\n        \tswitch (parsed) {\n\t\tcase REF_WORKTREE_CURRENT:\n\t\t\treturn WORKTREE_OTHER; /* iffy */\n                case REF_WORKTREE_MAIN:\n\t\t\treturn REF_WORKTREE_MAIN;\n\t\tcase REF_WORKTREE_OTHER:\n\t\tcase REF_WORKTREE_SHARED:\n\t\t\treturn parsed;\n\t\t}\n\t}\n\n\t/* Otherwise the input is like HEAD, MERGE_HEAD, refs/$BLAH */\n\t... do whatever for these trivial cases ...\n\n}\n\nAnd while composing this follow-up, I found another thing that is\niffy.  When you ask for \"worktrees/foo/refs/bisect/good\", the\nrecursive call in \"worktrees/foo\" for the remainder says \"current\",\nand the posted patch and the above pseudocode would take CURRENT to\nmean \"that other worktree's\", and turns it into OTHER i.e. \"not\nours\".  But the code does so without even checking what our worktree\nis called.  What happens if we are the owner of the \"worktrees/foo\"\nnamespace?  The same thing for \"The main worktree says the given ref\nis 'current', so we can turn that into 'main'\"---if our worktree is\n'main', it is also 'current' and does not have to be turned into\n'main' (even though it does not hurt).\n"},{"id":"462971","messageId":"xmqqillrb7qs.fsf@gitster.g","threadId":"58420","inReplyTo":"xmqq35cwcpws.fsf@gitster.g","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-13T15:43:07Z","receivedAt":"2022-09-13T16:51:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Otherwise we fall through with worktree_ref that we have stripped\n>> main-worktree/ prefix, which means the original input\n>>\n>> \tmain-worktree/worktrees/foo/blah\n>>\n>> is now \n>>\n>> \tworktrees/foo/blah\n>>\n>> and the next skip_prefix() will see that it begins with \"worktrees/\".\n>> Of course, if the initial input were\n>>\n>> \tworktrees/foo/blah\n>>\n>> then we wouldn't have skipped main-worktree/ prefix from it, and go\n>> to the next skip_prefix().  So from here on, we cannot tell which\n>> case the original input was.\n>>\n>> But that is OK.  Asking \"give me the ref 'blah' in the worktree 'foo'\"\n>> in the current worktree should yield the same answer to the question\n>> \"give me the ref 'blah' in the worktree 'foo', as if I asked you to\n>> do so in the main worktree\".\n>\n> This makes me wonder...\n>\n> I wonder if it makes the resulting code clearer to go fully\n> recursive, unlike the posted code that says \"if a recursive call\n> says it is for current, that means it is for main worktree, and\n> otherwise pretend as if the input did not have the prefix\".\n>\n> That is, something like\n> ...\n\nThe above may be a wrong suggestion, as it was solely guided by my\nreading of the posted code that looked as if it wanted to support\nsomething like \"main-worktree/worktrees/foo/refs/heads/main\".  If\nthat wasn't the intention, and we only want to support\n\n  1. a ref spelled in the traditional way before multiple worktree feature,\n  2. 1., with \"main-worktree/\" prefixed, or\n  3. 1., with \"worktrees/<worktreename>/\" prefixed\n\nthen a better approach would be to have a small helper\nparse_local_worktree_ref() and make the primary one into something\nlike\n\nparse_worktree_ref()\n{\n        if (begins with \"main-worktree/\") {\t\n\t\tstrip main-worktree/ prefix;\n\t\tswitch (parse_local_worktree_ref(input minus prefix)) {\n\t\t... the same switch to turn \"ours\" into \"main's\" ...\n\t\t... but we probably want to special case if we are ...\n\t\t... the main worktree ...\n\t\t}\n\t}\n\telse if (begins with \"worktrees/\") {\n\t\tstrip worktrees/ prefix and learn worktree name;\n\t\tswitch (parse_local_worktree_ref(input minus prefix)) {\n\t\t... the same switch to turn \"ours\" into \"theirs\" ...\n\t\t... but we probably want to special case if we are ...\n\t\t... the worktree in question ...\n\t\t}\n\t}\n\treturn parse_local_worktree_ref(input);\n}\n\nand have parse_local_worktree_ref() handle psuedorefs,\nper-worktree-refs.\n"},{"id":"463174","messageId":"CAFQ2z_PBWbdKgbaqLO6iXB8WEhG=CTjetrEgm7wHacDi_n8VHw@mail.gmail.com","threadId":"58420","inReplyTo":"xmqqillrb7qs.fsf@gitster.g","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-09-19T14:28:25Z","receivedAt":"2022-09-19T14:28:43Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Sep 13, 2022 at 5:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n> then a better approach would be to have a small helper\n> parse_local_worktree_ref() and make the primary one into something\n> like\n>...\n\nThanks, good idea. I'm sending you a v2.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Liana Sebastian\n"},{"id":"463192","messageId":"pull.1325.v2.git.git.1663605291172.gitgitgadget@gmail.com","threadId":"58420","inReplyTo":"pull.1325.git.git.1663002096207.gitgitgadget@gmail.com","subject":"[PATCH v2] refs: unify parse_worktree_ref() and ref_type()","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-09-19T16:34:50Z","receivedAt":"2022-09-19T16:35:04Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys <hanwen@google.com>\n\nThe logic to handle worktree refs (worktrees/NAME/REF and\nmain-worktree/REF) existed in two places:\n\n* ref_type() in refs.c\n\n* parse_worktree_ref() in worktree.c\n\nCollapse this logic together in one function parse_worktree_ref():\nthis avoids having to cross-check the result of parse_worktree_ref()\nand ref_type().\n\nIntroduce enum ref_worktree_type, which is slightly different from\nenum ref_type. The latter is a misleading name (one would think that\n'ref_type' would have the symref option).\n\nInstead, enum ref_worktree_type only makes explicit how a refname\nrelates to a worktree. From this point of view, HEAD and\nrefs/bisect/abc are the same: they specify the current worktree\nimplicitly.\n\nThe files-backend must avoid packing refs/bisect/* and friends into\npacked-refs, so expose is_per_worktree_ref() separately.\n\nSigned-off-by: Han-Wen Nienhuys <hanwen@google.com>\n---\n    refs: unify parse_worktree_ref() and ref_type()\n    \n    The logic to handle worktree refs (worktrees/NAME/REF and\n    main-worktree/REF) existed in two places:\n    \n     * ref_type() in refs.c\n    \n     * parse_worktree_ref() in worktree.c\n    \n    Collapse this logic together in one function parse_worktree_ref(): this\n    avoids having to cross-check the result of parse_worktree_ref() and\n    ref_type().\n    \n    Introduce enum ref_worktree_type, which is slightly different from enum\n    ref_type. The latter is a misleading name (one would think that\n    'ref_type' would have the symref option).\n    \n    Instead, enum ref_worktree_type only makes explicit how a refname\n    relates to a worktree. From this point of view, HEAD and refs/bisect/abc\n    are the same: they specify the current worktree implicitly.\n    \n    The files-backend must avoid packing refs/bisect/* and friends into\n    packed-refs, so expose is_per_worktree_ref() separately.\n    \n    Signed-off-by: Han-Wen Nienhuys hanwen@google.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1325%2Fhanwen%2Fparse-worktree-ref-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1325/hanwen/parse-worktree-ref-v2\nPull-Request: https://github.com/git/git/pull/1325\n\nRange-diff vs v1:\n\n 1:  e2d19b80f76 ! 1:  b1e6dc0edc7 refs: unify parse_worktree_ref() and ref_type()\n     @@ refs.c: static int is_pseudoref_syntax(const char *refname)\n       \t\t\treturn 0;\n       \t}\n       \n     -+\t/* HEAD is not a pseudoref, but it certainly uses the\n     -+\t * pseudoref syntax. */\n     ++\t/*\n     ++\t * HEAD is not a pseudoref, but it certainly uses the\n     ++\t * pseudoref syntax.\n     ++\t */\n       \treturn 1;\n       }\n       \n      -static int is_main_pseudoref_syntax(const char *refname)\n     -+enum ref_worktree_type parse_worktree_ref(const char *worktree_ref,\n     -+\t\t\t\t\t  const char **name, int *name_length,\n     -+\t\t\t\t\t  const char **ref)\n     - {\n     +-{\n      -\treturn skip_prefix(refname, \"main-worktree/\", &refname) &&\n      -\t\t*refname &&\n      -\t\tis_pseudoref_syntax(refname);\n     --}\n     -+\tconst char *name_dummy;\n     -+\tint name_length_dummy;\n     -+\tconst char *ref_dummy;\n     -+\tif (!name)\n     -+\t\tname = &name_dummy;\n     -+\tif (!name_length)\n     -+\t\tname_length = &name_length_dummy;\n     -+\tif (!ref)\n     -+\t\tref = &ref_dummy;\n     ++static int is_current_worktree_ref(const char *ref) {\n     ++\treturn is_pseudoref_syntax(ref) || is_per_worktree_ref(ref);\n     + }\n       \n      -static int is_other_pseudoref_syntax(const char *refname)\n     --{\n     ++enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n     ++\t\t\t\t\t  const char **worktree_name, int *worktree_name_length,\n     ++\t\t\t\t\t  const char **bare_refname)\n     + {\n      -\tif (!skip_prefix(refname, \"worktrees/\", &refname))\n      -\t\treturn 0;\n      -\trefname = strchr(refname, '/');\n     @@ refs.c: static int is_pseudoref_syntax(const char *refname)\n      -\t\treturn 0;\n      -\treturn is_pseudoref_syntax(refname + 1);\n      -}\n     -+\t*ref = worktree_ref;\n     -+\tif (is_pseudoref_syntax(worktree_ref)) {\n     -+\t\treturn REF_WORKTREE_CURRENT;\n     -+\t}\n     ++\tconst char *name_dummy;\n     ++\tint name_length_dummy;\n     ++\tconst char *ref_dummy;\n       \n      -enum ref_type ref_type(const char *refname)\n      -{\n     @@ refs.c: static int is_pseudoref_syntax(const char *refname)\n      -\tif (is_other_pseudoref_syntax(refname))\n      -\t\treturn REF_TYPE_OTHER_PSEUDOREF;\n      -\treturn REF_TYPE_NORMAL;\n     -+\tif (is_per_worktree_ref(worktree_ref)) {\n     -+\t\treturn REF_WORKTREE_CURRENT;\n     -+\t}\n     ++\tif (!worktree_name)\n     ++\t\tworktree_name = &name_dummy;\n     ++\tif (!worktree_name_length)\n     ++\t\tworktree_name_length = &name_length_dummy;\n     ++\tif (!bare_refname)\n     ++\t\tbare_refname = &ref_dummy;\n      +\n     -+\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n     -+\t\tif (!*worktree_ref)\n     -+\t\t\treturn -1;\n     -+\t\t*name = NULL;\n     -+\t\t*name_length = 0;\n     -+\t\t*ref = worktree_ref;\n     ++\tif (skip_prefix(maybe_worktree_ref, \"worktrees/\", bare_refname)) {\n     ++\t\tconst char *slash = strchr(*bare_refname, '/');\n      +\n     -+\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n     -+\t\t    REF_WORKTREE_CURRENT)\n     -+\t\t\treturn REF_WORKTREE_MAIN;\n     -+\t}\n     -+\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n     -+\t\tconst char *slash = strchr(worktree_ref, '/');\n     ++\t\t*worktree_name = *bare_refname;\n     ++\t\tif (!slash) {\n     ++\t\t\t*worktree_name_length = strlen(*worktree_name);\n      +\n     -+\t\tif (!slash || slash == worktree_ref || !slash[1])\n     -+\t\t\treturn -1;\n     -+\t\t*name = worktree_ref;\n     -+\t\t*name_length = slash - worktree_ref;\n     -+\t\t*ref = slash + 1;\n     ++\t\t\t/* This is an error condition, and the caller tell because the bare_refname is \"\" */\n     ++\t\t\t*bare_refname = *worktree_name + *worktree_name_length;\n     ++\t\t\treturn REF_WORKTREE_OTHER;\n     ++\t\t}\n     ++\n     ++\t\t*worktree_name_length = slash - *bare_refname;\n     ++\t\t*bare_refname = slash + 1;\n      +\n     -+\t\tif (parse_worktree_ref(*ref, NULL, NULL, NULL) ==\n     -+\t\t    REF_WORKTREE_CURRENT)\n     ++\t\tif (is_current_worktree_ref(*bare_refname))\n      +\t\t\treturn REF_WORKTREE_OTHER;\n      +\t}\n      +\n     ++\t*worktree_name = NULL;\n     ++\t*worktree_name_length = 0;\n     ++\n     ++\tif (skip_prefix(maybe_worktree_ref, \"main-worktree/\", bare_refname)\n     ++\t    && is_current_worktree_ref(*bare_refname))\n     ++\t\treturn REF_WORKTREE_MAIN;\n     ++\n     ++\t*bare_refname = maybe_worktree_ref;\n     ++\tif (is_current_worktree_ref(maybe_worktree_ref))\n     ++\t\treturn REF_WORKTREE_CURRENT;\n     ++\n      +\treturn REF_WORKTREE_SHARED;\n       }\n       \n     @@ refs.h: int parse_hide_refs_config(const char *var, const char *value, const cha\n      +\tREF_WORKTREE_CURRENT, /* implicitly per worktree, eg. HEAD or\n      +\t\t\t\t refs/bisect/SOMETHING */\n      +\tREF_WORKTREE_MAIN, /* explicitly in main worktree, eg.\n     -+\t\t\t      refs/main-worktree/HEAD */\n     ++\t\t\t      main-worktree/HEAD */\n      +\tREF_WORKTREE_OTHER, /* explicitly in named worktree, eg.\n     -+\t\t\t       refs/worktrees/bla/HEAD */\n     ++\t\t\t       worktrees/bla/HEAD */\n      +\tREF_WORKTREE_SHARED, /* the default, eg. refs/heads/main */\n       };\n       \n      -enum ref_type ref_type(const char *refname);\n     -+/* Parse a ref that possibly explicitly refers to a worktree ref\n     -+ * (ie. either REFNAME, main-worktree/REFNAME or\n     -+ * worktree/WORKTREE/REFNAME). If the name references a worktree\n     -+ * implicitly or explicitly, return what kind it was. The\n     -+ * worktree_name, worktree_name_length and refname argument maybe NULL.\n     ++/*\n     ++ * Parse a `maybe_worktree_ref` as a ref that possibly refers to a worktree ref\n     ++ * (ie. either REFNAME, main-worktree/REFNAME or worktree/WORKTREE/REFNAME). It\n     ++ * returns what kind of ref was found, and in case of REF_WORKTREE_OTHER, the\n     ++ * worktree name is returned in `worktree_name` (pointing into\n     ++ * `maybe_worktree_ref`) and `worktree_name_length`. The bare refname (the\n     ++ * refname stripped of prefixes) is returned in `bare_refname`. The\n     ++ * `worktree_name`, `worktree_name_length` and `bare_refname` arguments may be\n     ++ * NULL.\n      + */\n      +enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n      +\t\t\t\t\t  const char **worktree_name,\n      +\t\t\t\t\t  int *worktree_name_length,\n     -+\t\t\t\t\t  const char **refname);\n     ++\t\t\t\t\t  const char **bare_refname);\n       \n       enum expire_reflog_flags {\n       \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n\n\n builtin/reflog.c      |  3 +-\n reflog.c              | 13 ++-----\n refs.c                | 76 ++++++++++++++++++++++++++--------------\n refs.h                | 33 ++++++++++++++----\n refs/files-backend.c  | 80 +++++++++++++++++++------------------------\n refs/packed-backend.c |  2 +-\n worktree.c            | 59 ++++---------------------------\n worktree.h            | 10 ------\n 8 files changed, 126 insertions(+), 150 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 9407f835cb6..bd568d2d931 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -26,7 +26,8 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus\n \t * Avoid collecting the same shared ref multiple times because\n \t * they are available via all worktrees.\n \t */\n-\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n+\tif (!worktree->is_current &&\n+\t    parse_worktree_ref(ref, NULL, NULL, NULL) == REF_WORKTREE_SHARED)\n \t\treturn 0;\n \n \tstrbuf_worktree_ref(worktree, &newref, ref);\ndiff --git a/reflog.c b/reflog.c\nindex 47ba8620c56..0b8b767f97c 100644\n--- a/reflog.c\n+++ b/reflog.c\n@@ -310,16 +310,9 @@ static int push_tip_to_list(const char *refname, const struct object_id *oid,\n \n static int is_head(const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tif (parse_worktree_ref(refname, NULL, NULL, &refname))\n-\t\t\tBUG(\"not a worktree ref: %s\", refname);\n-\t\tbreak;\n-\tdefault:\n-\t\tbreak;\n-\t}\n-\treturn !strcmp(refname, \"HEAD\");\n+\tconst char *stripped_refname;\n+\tparse_worktree_ref(refname, NULL, NULL, &stripped_refname);\n+\treturn !strcmp(stripped_refname, \"HEAD\");\n }\n \n void reflog_expiry_prepare(const char *refname,\ndiff --git a/refs.c b/refs.c\nindex 1a964505f92..6e86607f3bb 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -722,7 +722,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)\n \treturn repo_dwim_log(the_repository, str, len, oid, log);\n }\n \n-static int is_per_worktree_ref(const char *refname)\n+int is_per_worktree_ref(const char *refname)\n {\n \treturn starts_with(refname, \"refs/worktree/\") ||\n \t       starts_with(refname, \"refs/bisect/\") ||\n@@ -738,37 +738,63 @@ static int is_pseudoref_syntax(const char *refname)\n \t\t\treturn 0;\n \t}\n \n+\t/*\n+\t * HEAD is not a pseudoref, but it certainly uses the\n+\t * pseudoref syntax.\n+\t */\n \treturn 1;\n }\n \n-static int is_main_pseudoref_syntax(const char *refname)\n-{\n-\treturn skip_prefix(refname, \"main-worktree/\", &refname) &&\n-\t\t*refname &&\n-\t\tis_pseudoref_syntax(refname);\n+static int is_current_worktree_ref(const char *ref) {\n+\treturn is_pseudoref_syntax(ref) || is_per_worktree_ref(ref);\n }\n \n-static int is_other_pseudoref_syntax(const char *refname)\n+enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n+\t\t\t\t\t  const char **worktree_name, int *worktree_name_length,\n+\t\t\t\t\t  const char **bare_refname)\n {\n-\tif (!skip_prefix(refname, \"worktrees/\", &refname))\n-\t\treturn 0;\n-\trefname = strchr(refname, '/');\n-\tif (!refname || !refname[1])\n-\t\treturn 0;\n-\treturn is_pseudoref_syntax(refname + 1);\n-}\n+\tconst char *name_dummy;\n+\tint name_length_dummy;\n+\tconst char *ref_dummy;\n \n-enum ref_type ref_type(const char *refname)\n-{\n-\tif (is_per_worktree_ref(refname))\n-\t\treturn REF_TYPE_PER_WORKTREE;\n-\tif (is_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_PSEUDOREF;\n-\tif (is_main_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_MAIN_PSEUDOREF;\n-\tif (is_other_pseudoref_syntax(refname))\n-\t\treturn REF_TYPE_OTHER_PSEUDOREF;\n-\treturn REF_TYPE_NORMAL;\n+\tif (!worktree_name)\n+\t\tworktree_name = &name_dummy;\n+\tif (!worktree_name_length)\n+\t\tworktree_name_length = &name_length_dummy;\n+\tif (!bare_refname)\n+\t\tbare_refname = &ref_dummy;\n+\n+\tif (skip_prefix(maybe_worktree_ref, \"worktrees/\", bare_refname)) {\n+\t\tconst char *slash = strchr(*bare_refname, '/');\n+\n+\t\t*worktree_name = *bare_refname;\n+\t\tif (!slash) {\n+\t\t\t*worktree_name_length = strlen(*worktree_name);\n+\n+\t\t\t/* This is an error condition, and the caller tell because the bare_refname is \"\" */\n+\t\t\t*bare_refname = *worktree_name + *worktree_name_length;\n+\t\t\treturn REF_WORKTREE_OTHER;\n+\t\t}\n+\n+\t\t*worktree_name_length = slash - *bare_refname;\n+\t\t*bare_refname = slash + 1;\n+\n+\t\tif (is_current_worktree_ref(*bare_refname))\n+\t\t\treturn REF_WORKTREE_OTHER;\n+\t}\n+\n+\t*worktree_name = NULL;\n+\t*worktree_name_length = 0;\n+\n+\tif (skip_prefix(maybe_worktree_ref, \"main-worktree/\", bare_refname)\n+\t    && is_current_worktree_ref(*bare_refname))\n+\t\treturn REF_WORKTREE_MAIN;\n+\n+\t*bare_refname = maybe_worktree_ref;\n+\tif (is_current_worktree_ref(maybe_worktree_ref))\n+\t\treturn REF_WORKTREE_CURRENT;\n+\n+\treturn REF_WORKTREE_SHARED;\n }\n \n long get_files_ref_lock_timeout_ms(void)\ndiff --git a/refs.h b/refs.h\nindex 23479c7ee09..a42957a7917 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -825,15 +825,34 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);\n  */\n int ref_is_hidden(const char *, const char *);\n \n-enum ref_type {\n-\tREF_TYPE_PER_WORKTREE,\t  /* refs inside refs/ but not shared       */\n-\tREF_TYPE_PSEUDOREF,\t  /* refs outside refs/ in current worktree */\n-\tREF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */\n-\tREF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */\n-\tREF_TYPE_NORMAL,\t  /* normal/shared refs inside refs/        */\n+/* Is this a per-worktree ref living in the refs/ namespace? */\n+int is_per_worktree_ref(const char *refname);\n+\n+/* Describes how a refname relates to worktrees */\n+enum ref_worktree_type {\n+\tREF_WORKTREE_CURRENT, /* implicitly per worktree, eg. HEAD or\n+\t\t\t\t refs/bisect/SOMETHING */\n+\tREF_WORKTREE_MAIN, /* explicitly in main worktree, eg.\n+\t\t\t      main-worktree/HEAD */\n+\tREF_WORKTREE_OTHER, /* explicitly in named worktree, eg.\n+\t\t\t       worktrees/bla/HEAD */\n+\tREF_WORKTREE_SHARED, /* the default, eg. refs/heads/main */\n };\n \n-enum ref_type ref_type(const char *refname);\n+/*\n+ * Parse a `maybe_worktree_ref` as a ref that possibly refers to a worktree ref\n+ * (ie. either REFNAME, main-worktree/REFNAME or worktree/WORKTREE/REFNAME). It\n+ * returns what kind of ref was found, and in case of REF_WORKTREE_OTHER, the\n+ * worktree name is returned in `worktree_name` (pointing into\n+ * `maybe_worktree_ref`) and `worktree_name_length`. The bare refname (the\n+ * refname stripped of prefixes) is returned in `bare_refname`. The\n+ * `worktree_name`, `worktree_name_length` and `bare_refname` arguments may be\n+ * NULL.\n+ */\n+enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,\n+\t\t\t\t\t  const char **worktree_name,\n+\t\t\t\t\t  int *worktree_name_length,\n+\t\t\t\t\t  const char **bare_refname);\n \n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 95acab78eef..f230704229e 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -138,44 +138,30 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,\n \treturn refs;\n }\n \n-static void files_reflog_path_other_worktrees(struct files_ref_store *refs,\n-\t\t\t\t\t      struct strbuf *sb,\n-\t\t\t\t\t      const char *refname)\n-{\n-\tconst char *real_ref;\n-\tconst char *worktree_name;\n-\tint length;\n-\n-\tif (parse_worktree_ref(refname, &worktree_name, &length, &real_ref))\n-\t\tBUG(\"refname %s is not a other-worktree ref\", refname);\n-\n-\tif (worktree_name)\n-\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/logs/%s\", refs->gitcommondir,\n-\t\t\t    length, worktree_name, real_ref);\n-\telse\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir,\n-\t\t\t    real_ref);\n-}\n-\n static void files_reflog_path(struct files_ref_store *refs,\n \t\t\t      struct strbuf *sb,\n \t\t\t      const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PER_WORKTREE:\n-\tcase REF_TYPE_PSEUDOREF:\n+\tconst char *bare_refname;\n+\tconst char *wtname;\n+\tint wtname_len;\n+\tenum ref_worktree_type wt_type = parse_worktree_ref(\n+\t\trefname, &wtname, &wtname_len, &bare_refname);\n+\n+\tswitch (wt_type) {\n+\tcase REF_WORKTREE_CURRENT:\n \t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->base.gitdir, refname);\n \t\tbreak;\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tfiles_reflog_path_other_worktrees(refs, sb, refname);\n+\tcase REF_WORKTREE_SHARED:\n+\tcase REF_WORKTREE_MAIN:\n+\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir, bare_refname);\n \t\tbreak;\n-\tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/logs/%s\", refs->gitcommondir, refname);\n+\tcase REF_WORKTREE_OTHER:\n+\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/logs/%s\", refs->gitcommondir,\n+\t\t\t    wtname_len, wtname, bare_refname);\n \t\tbreak;\n \tdefault:\n-\t\tBUG(\"unknown ref type %d of ref %s\",\n-\t\t    ref_type(refname), refname);\n+\t\tBUG(\"unknown ref type %d of ref %s\", wt_type, refname);\n \t}\n }\n \n@@ -183,22 +169,25 @@ static void files_ref_path(struct files_ref_store *refs,\n \t\t\t   struct strbuf *sb,\n \t\t\t   const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PER_WORKTREE:\n-\tcase REF_TYPE_PSEUDOREF:\n+\tconst char *bare_refname;\n+\tconst char *wtname;\n+\tint wtname_len;\n+\tenum ref_worktree_type wt_type = parse_worktree_ref(\n+\t\trefname, &wtname, &wtname_len, &bare_refname);\n+\tswitch (wt_type) {\n+\tcase REF_WORKTREE_CURRENT:\n \t\tstrbuf_addf(sb, \"%s/%s\", refs->base.gitdir, refname);\n \t\tbreak;\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\t\tif (!skip_prefix(refname, \"main-worktree/\", &refname))\n-\t\t\tBUG(\"ref %s is not a main pseudoref\", refname);\n-\t\t/* fallthrough */\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\tcase REF_TYPE_NORMAL:\n-\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitcommondir, refname);\n+\tcase REF_WORKTREE_OTHER:\n+\t\tstrbuf_addf(sb, \"%s/worktrees/%.*s/%s\", refs->gitcommondir,\n+\t\t\t    wtname_len, wtname, bare_refname);\n+\t\tbreak;\n+\tcase REF_WORKTREE_SHARED:\n+\tcase REF_WORKTREE_MAIN:\n+\t\tstrbuf_addf(sb, \"%s/%s\", refs->gitcommondir, bare_refname);\n \t\tbreak;\n \tdefault:\n-\t\tBUG(\"unknown ref type %d of ref %s\",\n-\t\t    ref_type(refname), refname);\n+\t\tBUG(\"unknown ref type %d of ref %s\", wt_type, refname);\n \t}\n }\n \n@@ -771,7 +760,8 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator)\n \n \twhile ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) {\n \t\tif (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&\n-\t\t    ref_type(iter->iter0->refname) != REF_TYPE_PER_WORKTREE)\n+\t\t    parse_worktree_ref(iter->iter0->refname, NULL, NULL,\n+\t\t\t\t       NULL) != REF_WORKTREE_CURRENT)\n \t\t\tcontinue;\n \n \t\tif ((iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS) &&\n@@ -1179,7 +1169,8 @@ static int should_pack_ref(const char *refname,\n \t\t\t   unsigned int pack_flags)\n {\n \t/* Do not pack per-worktree refs: */\n-\tif (ref_type(refname) != REF_TYPE_NORMAL)\n+\tif (parse_worktree_ref(refname, NULL, NULL, NULL) !=\n+\t    REF_WORKTREE_SHARED)\n \t\treturn 0;\n \n \t/* Do not pack non-tags unless PACK_REFS_ALL is set: */\n@@ -2277,7 +2268,8 @@ static enum iterator_selection reflog_iterator_select(\n \t\t */\n \t\treturn ITER_SELECT_0;\n \t} else if (iter_common) {\n-\t\tif (ref_type(iter_common->refname) == REF_TYPE_NORMAL)\n+\t\tif (parse_worktree_ref(iter_common->refname, NULL, NULL,\n+\t\t\t\t       NULL) == REF_WORKTREE_SHARED)\n \t\t\treturn ITER_SELECT_1;\n \n \t\t/*\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 66c4574c99d..bf0e63ae70c 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -862,7 +862,7 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)\n \n \twhile ((ok = next_record(iter)) == ITER_OK) {\n \t\tif (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&\n-\t\t    ref_type(iter->base.refname) != REF_TYPE_PER_WORKTREE)\n+\t\t    !is_per_worktree_ref(iter->base.refname))\n \t\t\tcontinue;\n \n \t\tif (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) &&\ndiff --git a/worktree.c b/worktree.c\nindex 90fc085f76b..bb7873c72d1 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -489,62 +489,17 @@ int submodule_uses_worktrees(const char *path)\n \treturn ret;\n }\n \n-int parse_worktree_ref(const char *worktree_ref, const char **name,\n-\t\t       int *name_length, const char **ref)\n-{\n-\tif (skip_prefix(worktree_ref, \"main-worktree/\", &worktree_ref)) {\n-\t\tif (!*worktree_ref)\n-\t\t\treturn -1;\n-\t\tif (name)\n-\t\t\t*name = NULL;\n-\t\tif (name_length)\n-\t\t\t*name_length = 0;\n-\t\tif (ref)\n-\t\t\t*ref = worktree_ref;\n-\t\treturn 0;\n-\t}\n-\tif (skip_prefix(worktree_ref, \"worktrees/\", &worktree_ref)) {\n-\t\tconst char *slash = strchr(worktree_ref, '/');\n-\n-\t\tif (!slash || slash == worktree_ref || !slash[1])\n-\t\t\treturn -1;\n-\t\tif (name)\n-\t\t\t*name = worktree_ref;\n-\t\tif (name_length)\n-\t\t\t*name_length = slash - worktree_ref;\n-\t\tif (ref)\n-\t\t\t*ref = slash + 1;\n-\t\treturn 0;\n-\t}\n-\treturn -1;\n-}\n-\n void strbuf_worktree_ref(const struct worktree *wt,\n \t\t\t struct strbuf *sb,\n \t\t\t const char *refname)\n {\n-\tswitch (ref_type(refname)) {\n-\tcase REF_TYPE_PSEUDOREF:\n-\tcase REF_TYPE_PER_WORKTREE:\n-\t\tif (wt && !wt->is_current) {\n-\t\t\tif (is_main_worktree(wt))\n-\t\t\t\tstrbuf_addstr(sb, \"main-worktree/\");\n-\t\t\telse\n-\t\t\t\tstrbuf_addf(sb, \"worktrees/%s/\", wt->id);\n-\t\t}\n-\t\tbreak;\n-\n-\tcase REF_TYPE_MAIN_PSEUDOREF:\n-\tcase REF_TYPE_OTHER_PSEUDOREF:\n-\t\tbreak;\n-\n-\tcase REF_TYPE_NORMAL:\n-\t\t/*\n-\t\t * For shared refs, don't prefix worktrees/ or\n-\t\t * main-worktree/. It's not necessary and\n-\t\t * files-backend.c can't handle it anyway.\n-\t\t */\n-\t\tbreak;\n+\tif (parse_worktree_ref(refname, NULL, NULL, NULL) ==\n+\t\t    REF_WORKTREE_CURRENT &&\n+\t    wt && !wt->is_current) {\n+\t\tif (is_main_worktree(wt))\n+\t\t\tstrbuf_addstr(sb, \"main-worktree/\");\n+\t\telse\n+\t\t\tstrbuf_addf(sb, \"worktrees/%s/\", wt->id);\n \t}\n \tstrbuf_addstr(sb, refname);\n }\ndiff --git a/worktree.h b/worktree.h\nindex e9e839926b0..9dcea6fc8c1 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -166,16 +166,6 @@ const char *worktree_git_path(const struct worktree *wt,\n \t\t\t      const char *fmt, ...)\n \t__attribute__((format (printf, 2, 3)));\n \n-/*\n- * Parse a worktree ref (i.e. with prefix main-worktree/ or\n- * worktrees/) and return the position of the worktree's name and\n- * length (or NULL and zero if it's main worktree), and ref.\n- *\n- * All name, name_length and ref arguments could be NULL.\n- */\n-int parse_worktree_ref(const char *worktree_ref, const char **name,\n-\t\t       int *name_length, const char **ref);\n-\n /*\n  * Return a refname suitable for access from the current ref store.\n  */\n\nbase-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n-- \ngitgitgadget\n"},{"id":"463236","messageId":"xmqqwn9z82hk.fsf@gitster.g","threadId":"58420","inReplyTo":"CAFQ2z_PBWbdKgbaqLO6iXB8WEhG=CTjetrEgm7wHacDi_n8VHw@mail.gmail.com","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-19T21:43:03Z","receivedAt":"2022-09-19T21:43:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys <hanwen@google.com> writes:\n\n> On Tue, Sep 13, 2022 at 5:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> then a better approach would be to have a small helper\n>> parse_local_worktree_ref() and make the primary one into something\n>> like\n>>...\n>\n> Thanks, good idea. I'm sending you a v2.\n\nHmph, is that \"v2\" <pull.1325.v2.git.git.1663605291172.gitgitgadget@gmail.com>\nor is there another version of it?\n"},{"id":"463294","messageId":"CAFQ2z_PQFtq-ph1B0tUFDW7ngVwg9++k2M_5rvozsLVisX2+Qg@mail.gmail.com","threadId":"58420","inReplyTo":"xmqqwn9z82hk.fsf@gitster.g","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-09-20T08:53:59Z","receivedAt":"2022-09-20T08:54:18Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Sep 19, 2022 at 11:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > On Tue, Sep 13, 2022 at 5:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n> >> then a better approach would be to have a small helper\n> >> parse_local_worktree_ref() and make the primary one into something\n> >> like\n> >>...\n> >\n> > Thanks, good idea. I'm sending you a v2.\n>\n> is that \"v2\" <pull.1325.v2.git.git.1663605291172.gitgitgadget@gmail.com>\n> or is there another version of it?\n\nI think so.\n\n> Hmph,\n\nDid I do something wrong?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\nRegistergericht und -nummer: Hamburg, HRB 86891\nSitz der Gesellschaft: Hamburg\nGeschäftsführer: Paul Manicle, Liana Sebastian\n"},{"id":"463375","messageId":"xmqqczbo65h8.fsf@gitster.g","threadId":"58420","inReplyTo":"CAFQ2z_PQFtq-ph1B0tUFDW7ngVwg9++k2M_5rvozsLVisX2+Qg@mail.gmail.com","subject":"Re: [PATCH] refs: unify parse_worktree_ref() and ref_type()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-21T16:45:55Z","receivedAt":"2022-09-21T16:50:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys <hanwen@google.com> writes:\n\n> On Mon, Sep 19, 2022 at 11:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> > On Tue, Sep 13, 2022 at 5:43 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> >> then a better approach would be to have a small helper\n>> >> parse_local_worktree_ref() and make the primary one into something\n>> >> like\n>> >>...\n>> >\n>> > Thanks, good idea. I'm sending you a v2.\n>>\n>> is that \"v2\" <pull.1325.v2.git.git.1663605291172.gitgitgadget@gmail.com>\n>> or is there another version of it?\n>\n> I think so.\n>\n>> Hmph,\n>\n> Did I do something wrong?\n\nNot \"wrong\" per-se, but I was surprised by the patch that looked\nquite different from what I expected from your response to the\n\"small helper\" suggestion.\n"}]}