Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
- From
ZheNing Hu <adlternative@gmail.com>
- Date
- Jul 22, 2021, 09:10 UTC
- Message-ID
- <CAOLTT8RULMLiUwRDMRO-W6gzB8+57XHT=Hn3owoQF-fyK8_HMw@mail.gmail.com>
- In-Reply-To
- <874kcmvjr6.fsf@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2021年7月22日周四 下午4:24写道：
Show 27 quoted lines
>
>
> On Thu, Jul 22 2021, ZheNing Hu via GitGitGadget wrote:
>
>
> > + } 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]
Show 127 quoted lines
>
> 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