Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> If you look closely, thats only for the local branches, the remotes
> have `align` atom when
> printing in verbose.
Yes, but that's already one thing factored out of the if, even if it's
just for local.
Actually, I think you can also factor some parts out of the
%(if:notequals=remotes). In 'local', you have an %(if) to display either
"* " or " ", and in remote you always start with " ". Why not always
apply the %(if), and let it display " " if not displaying the current
branch? Similarly, the "verbose" part of remote branches seems like a
particular case of the one for local ones (remotes don't have tracking
branches, so the tracking info should expand to the empty string).
To go a bit further, you can pre-build a string or strbuf aligned_short
with value like "%%(align:20,left)%%(refname:short)%%(end)" and use it
where needed (it's not a constant because you have to introduce maxwidth
into the string, so it's not a candidate for #define).
> I could cook up this:
Your mailer broke the formatting, so it looks terrible, but from what I
could parse, it's already much better than the previous one. It's not a
matter of size of the function, I very much prefer reading 10 lines of
nice code than 4 lines like
> + local = xstrfmt("%%(if)%%(HEAD)%%(then)* %s%%(else) %%(end)%%(align:%d,left)%%(refname:short)%%(end)%s"
> + " %%(objectname:short,7) %%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s] %%(end)"
> + "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)",
> + branch_get_color(BRANCH_COLOR_CURRENT), maxwidth, branch_get_color(BRANCH_COLOR_RESET),
> + branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
;-).
One obvious issue with the initial version was this big hard-to-parse
block, but another one is that the code did not make it easy to
understand what was changing depending on which branch of the if, and
depending on local/remote. It's getting much easier already.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html