Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
From: ZheNing Hu <hidden>
Date: 2021-07-22 09:10:28
Ævar Arnfjörð Bjarmason [off-list ref] 于2021年7月22日周四 下午4:24写道：
On Thu, Jul 22 2021, ZheNing Hu via GitGitGadget wrote:quoted
+ } else if (atom_type == ATOM_REST) { + if (ref->rest) + v->s = xstrdup(ref->rest); + else + v->s = xstrdup(""); + continue; } else continue;Another light reading nit, maybe more readable as: } else if (atom_type == ATOM_REST && ref->rest) { ... } else if (atom_type == ATOM_REST) { ... } continue; But we can't do that "continue" at the end because there's two cases where we don't continue, I wondered if elimanting that special case would make it easier, patch below. Probably not worth it & feel free to ignore:
Yeah, the readability here is really bad, so many "continue" just to not execute part of the operation on refname. In fact, I have a similar patch (which will not be sent to the mailing list for the time being), it use switch and case to replace if and else. [1]
quoted hunk ↗ jump to hunk
ref-filter.c | 63 +++++++++++++++++++++++++----------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-)diff --git a/ref-filter.c b/ref-filter.c index 81e77b13ad2..189244fed6f 100644 --- a/ref-filter.c +++ b/ref-filter.c@@ -1890,18 +1890,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) name++; } - if (atom_type == ATOM_REFNAME) + if (atom_type == ATOM_REFNAME) { refname = get_refname(atom, ref); - else if (atom_type == ATOM_WORKTREEPATH) { - if (ref->kind == FILTER_REFS_BRANCHES) - v->s = get_worktree_path(atom, ref); + if (!deref) + v->s = xstrdup(refname); else - v->s = xstrdup(""); - continue; - } - else if (atom_type == ATOM_SYMREF) + v->s = xstrfmt("%s^{}", refname); + free((char *)refname); + } else if (atom_type == ATOM_WORKTREEPATH && + ref->kind == FILTER_REFS_BRANCHES) { + v->s = get_worktree_path(atom, ref); + } else if (atom_type == ATOM_WORKTREEPATH) { + v->s = xstrdup(""); + } else if (atom_type == ATOM_SYMREF) { refname = get_symref(atom, ref); - else if (atom_type == ATOM_UPSTREAM) { + if (!deref) + v->s = xstrdup(refname); + else + v->s = xstrfmt("%s^{}", refname); + free((char *)refname); + } else if (atom_type == ATOM_UPSTREAM) { const char *branch_name; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/",@@ -1916,7 +1924,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) fill_remote_ref_details(atom, refname, branch, &v->s); else v->s = xstrdup(""); - continue; } else if (atom_type == ATOM_PUSH && atom->u.remote_ref.push) { const char *branch_name; v->s = xstrdup("");@@ -1935,10 +1942,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) /* We will definitely re-init v->s on the next line. */ free((char *)v->s); fill_remote_ref_details(atom, refname, branch, &v->s); - continue; } else if (atom_type == ATOM_COLOR) { v->s = xstrdup(atom->u.color); - continue; } else if (atom_type == ATOM_FLAG) { char buf[256], *cp = buf; if (ref->flag & REF_ISSYMREF)@@ -1951,24 +1956,20 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) *cp = '\0'; v->s = xstrdup(buf + 1); } - continue; } else if (!deref && atom_type == ATOM_OBJECTNAME) { v->s = xstrdup(do_grab_oid("objectname", &ref->objectname, atom)); - continue; + } else if (atom_type == ATOM_HEAD && + atom->u.head && + !strcmp(ref->refname, atom->u.head)) { + v->s = xstrdup("*"); } else if (atom_type == ATOM_HEAD) { - if (atom->u.head && !strcmp(ref->refname, atom->u.head)) - v->s = xstrdup("*"); - else - v->s = xstrdup(" "); - continue; + v->s = xstrdup(" "); } else if (atom_type == ATOM_ALIGN) { v->handler = align_atom_handler; v->s = xstrdup(""); - continue; } else if (atom_type == ATOM_END) { v->handler = end_atom_handler; v->s = xstrdup(""); - continue; } else if (atom_type == ATOM_IF) { const char *s; if (skip_prefix(name, "if:", &s))@@ -1976,29 +1977,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) else v->s = xstrdup(""); v->handler = if_atom_handler; - continue; } else if (atom_type == ATOM_THEN) { v->handler = then_atom_handler; v->s = xstrdup(""); - continue; } else if (atom_type == ATOM_ELSE) { v->handler = else_atom_handler; v->s = xstrdup(""); - continue; + } else if (atom_type == ATOM_REST && ref->rest) { + v->s = xstrdup(ref->rest); } else if (atom_type == ATOM_REST) { - if (ref->rest) - v->s = xstrdup(ref->rest); - else - v->s = xstrdup(""); - continue; - } else - continue; - - if (!deref) - v->s = xstrdup(refname); - else - v->s = xstrfmt("%s^{}", refname); - free((char *)refname); + v->s = xstrdup(""); + } } for (i = 0; i < used_atom_cnt; i++) {
[1]: https://github.com/adlternative/git/commit/bbc6ecba4c0f65e7328e571b0d38ff99f800dc09