Junio writes [1]: > I even wonder why dwim_ref() is not defined like so in a header: > > #define dwim_ref(s, l, o, r) \ > repo_dwim_ref(the_repository, (s), (l), (o), (r)) > > Which would force a change like the one we are discussing to keep > them in parallel and keep the promise that only difference between > the two is that dwim_ref() works with the_repository and the other > can take an arbitrary repository. Perhaps that can be a preliminary > clean-up patch before these two patches ;-) OK done - that's patch 2 here. (I used "static inline" instead of a preprocessor #define because I prefer not to use the preprocessor whenever possible, but switching to #define is fine too.) > Reducing the size of the diff is a good justification only when the > end result is the same. If it burdens future developers more, that > is "I write less at the expense of forcing others to write more", > which is not quite the same thing. OK - that makes sense. [1] https://lore.kernel.org/git/xmqqzh6ag1ih.fsf@xxxxxxxxxxxxxxxxxxxxxx/ Jonathan Tan (3): sha1-name: replace unsigned int with option struct refs: move dwim_ref() to header file wt-status: tolerate dangling marks archive.c | 4 ++-- branch.c | 2 +- builtin/checkout.c | 4 ++-- builtin/fast-export.c | 2 +- builtin/log.c | 2 +- builtin/merge.c | 2 +- builtin/reset.c | 2 +- builtin/rev-parse.c | 2 +- builtin/show-branch.c | 2 +- builtin/stash.c | 2 +- bundle.c | 2 +- cache.h | 27 ++++++++++++++++++-------- commit.c | 2 +- refs.c | 20 +++++++++---------- refs.h | 12 ++++++++++-- remote.c | 2 +- revision.c | 3 ++- sha1-name.c | 45 ++++++++++++++++++++++++++++--------------- t/t7508-status.sh | 12 ++++++++++++ wt-status.c | 2 +- 20 files changed, 98 insertions(+), 53 deletions(-) Range-diff against v2: -: ---------- > 1: 6b3e3077e6 refs: move dwim_ref() to header file 1: 59b91a206d ! 2: 8f489d9633 wt-status: tolerate dangling marks @@ Commit message Therefore, when calculating the status of a worktree, tolerate dangling marks. This is done by adding an additional parameter to - repo_dwim_ref(). + dwim_ref() and repo_dwim_ref(). Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> + ## archive.c ## +@@ archive.c: static void parse_treeish_arg(const char **argv, + const char *colon = strchrnul(name, ':'); + int refnamelen = colon - name; + +- if (!dwim_ref(name, refnamelen, &oid, &ref)) ++ if (!dwim_ref(name, refnamelen, &oid, &ref, 0)) + die(_("no such ref: %.*s"), refnamelen, name); + } else { +- dwim_ref(name, strlen(name), &oid, &ref); ++ dwim_ref(name, strlen(name), &oid, &ref, 0); + } + + if (get_oid(name, &oid)) + + ## branch.c ## +@@ branch.c: void create_branch(struct repository *r, + die(_("Not a valid object name: '%s'."), start_name); + } + +- switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref)) { ++ switch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) { + case 0: + /* Not branching from any existing branch */ + if (explicit_tracking) + + ## builtin/checkout.c ## +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch) + * If this is a ref, resolve it; otherwise, look up the OID for our + * expression. Failure here is okay. + */ +- if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname)) ++ if (!dwim_ref(branch->name, strlen(branch->name), &branch->oid, &branch->refname, 0)) + repo_get_oid_committish(the_repository, branch->name, &branch->oid); + + strbuf_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL); +@@ builtin/checkout.c: static void die_expecting_a_branch(const struct branch_info *branch_info) + struct object_id oid; + char *to_free; + +- if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free) == 1) { ++ if (dwim_ref(branch_info->name, strlen(branch_info->name), &oid, &to_free, 0) == 1) { + const char *ref = to_free; + + if (skip_prefix(ref, "refs/tags/", &ref)) + + ## builtin/fast-export.c ## +@@ builtin/fast-export.c: static void get_tags_and_duplicates(struct rev_cmdline_info *info) + if (e->flags & UNINTERESTING) + continue; + +- if (dwim_ref(e->name, strlen(e->name), &oid, &full_name) != 1) ++ if (dwim_ref(e->name, strlen(e->name), &oid, &full_name, 0) != 1) + continue; + + if (refspecs.nr) { + + ## builtin/log.c ## +@@ builtin/log.c: static char *find_branch_name(struct rev_info *rev) + return NULL; + ref = rev->cmdline.rev[positive].name; + tip_oid = &rev->cmdline.rev[positive].item->oid; +- if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref) && ++ if (dwim_ref(ref, strlen(ref), &branch_oid, &full_ref, 0) && + skip_prefix(full_ref, "refs/heads/", &v) && + oideq(tip_oid, &branch_oid)) + branch = xstrdup(v); + + ## builtin/merge.c ## +@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg) + if (!remote_head) + die(_("'%s' does not point to a commit"), remote); + +- if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref) > 0) { ++ if (dwim_ref(remote, strlen(remote), &branch_head, &found_ref, 0) > 0) { + if (starts_with(found_ref, "refs/heads/")) { + strbuf_addf(msg, "%s\t\tbranch '%s' of .\n", + oid_to_hex(&branch_head), remote); + + ## builtin/reset.c ## +@@ builtin/reset.c: int cmd_reset(int argc, const char **argv, const char *prefix) + char *ref = NULL; + int err; + +- dwim_ref(rev, strlen(rev), &dummy, &ref); ++ dwim_ref(rev, strlen(rev), &dummy, &ref, 0); + if (ref && !starts_with(ref, "refs/")) + ref = NULL; + + + ## builtin/rev-parse.c ## +@@ builtin/rev-parse.c: static void show_rev(int type, const struct object_id *oid, const char *name) + struct object_id discard; + char *full; + +- switch (dwim_ref(name, strlen(name), &discard, &full)) { ++ switch (dwim_ref(name, strlen(name), &discard, &full, 0)) { + case 0: + /* + * Not found -- not a ref. We could + + ## builtin/show-branch.c ## +@@ builtin/show-branch.c: int cmd_show_branch(int ac, const char **av, const char *prefix) + die(Q_("only %d entry can be shown at one time.", + "only %d entries can be shown at one time.", + MAX_REVS), MAX_REVS); +- if (!dwim_ref(*av, strlen(*av), &oid, &ref)) ++ if (!dwim_ref(*av, strlen(*av), &oid, &ref, 0)) + die(_("no such ref %s"), *av); + + /* Has the base been specified? */ + + ## builtin/stash.c ## +@@ builtin/stash.c: static int get_stash_info(struct stash_info *info, int argc, const char **argv) + end_of_rev = strchrnul(revision, '@'); + strbuf_add(&symbolic, revision, end_of_rev - revision); + +- ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref); ++ ret = dwim_ref(symbolic.buf, symbolic.len, &dummy, &expanded_ref, 0); + strbuf_release(&symbolic); + switch (ret) { + case 0: /* Not found, but valid ref */ + + ## bundle.c ## +@@ bundle.c: static int write_bundle_refs(int bundle_fd, struct rev_info *revs) + + if (e->item->flags & UNINTERESTING) + continue; +- if (dwim_ref(e->name, strlen(e->name), &oid, &ref) != 1) ++ if (dwim_ref(e->name, strlen(e->name), &oid, &ref, 0) != 1) + goto skip_write_ref; + if (read_ref_full(e->name, RESOLVE_REF_READING, &oid, &flag)) + flag = 0; + ## cache.h ## @@ cache.h: struct interpret_branch_name_options { * allowed, even ones to refs outside of those namespaces. @@ cache.h: struct interpret_branch_name_options { int repo_interpret_branch_name(struct repository *r, const char *str, int len, + ## commit.c ## +@@ commit.c: struct commit *get_fork_point(const char *refname, struct commit *commit) + struct commit *ret = NULL; + char *full_refname; + +- switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) { ++ switch (dwim_ref(refname, strlen(refname), &oid, &full_refname, 0)) { + case 0: + die("No such ref: '%s'", refname); + case 1: + ## refs.c ## @@ refs.c: const char *git_default_branch_name(void) * to name a branch. @@ refs.c: static char *substitute_branch_name(struct repository *r, int refs_found = expand_ref(r, str, len, oid, ref); free(last_branch); return refs_found; -@@ refs.c: int repo_dwim_ref(struct repository *r, const char *str, int len, - - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref) - { -- return repo_dwim_ref(the_repository, str, len, oid, ref); -+ return repo_dwim_ref(the_repository, str, len, oid, ref, 0); - } - - int expand_ref(struct repository *repo, const char *str, int len, @@ refs.c: int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **log) { @@ refs.h: struct strvec; +int repo_dwim_ref(struct repository *r, const char *str, int len, + struct object_id *oid, char **ref, int nonfatal_dangling_mark); int repo_dwim_log(struct repository *r, const char *str, int len, struct object_id *oid, char **ref); - int dwim_ref(const char *str, int len, struct object_id *oid, char **ref); + static inline int dwim_ref(const char *str, int len, struct object_id *oid, +- char **ref) ++ char **ref, int nonfatal_dangling_mark) + { +- return repo_dwim_ref(the_repository, str, len, oid, ref); ++ return repo_dwim_ref(the_repository, str, len, oid, ref, ++ nonfatal_dangling_mark); + } int dwim_log(const char *str, int len, struct object_id *oid, char **ref); + + + ## remote.c ## +@@ remote.c: static void set_merge(struct branch *ret) + strcmp(ret->remote_name, ".")) + continue; + if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]), +- &oid, &ref) == 1) ++ &oid, &ref, 0) == 1) + ret->merge[i]->dst = ref; + else + ret->merge[i]->dst = xstrdup(ret->merge_name[i]); ## sha1-name.c ## @@ sha1-name.c: static int get_oid_basic(struct repository *r, const char *str, int len, @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r, } - if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref) == 1 && -+ if (repo_dwim_ref(the_repository, cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 && ++ if (dwim_ref(cb.buf.buf, cb.buf.len, &oid, &ref, 1) == 1 && /* sha1 is a commit? match without further lookup */ (oideq(&cb.noid, &oid) || /* perhaps sha1 is a tag, try to dereference to a commit */ -- 2.28.0.402.g5ffc5be6b7-goog
