Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom

[Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>]

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:

 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++) {