Jeff King <peff@xxxxxxxx> writes: > On Thu, Mar 23, 2017 at 08:18:26PM +0100, René Scharfe wrote: > >> Am 23.03.2017 um 16:50 schrieb SZEDER Gábor: >> > This eliminates three magic numbers. >> > >> > Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx> >> > --- >> > refs.c | 10 +++++----- >> > 1 file changed, 5 insertions(+), 5 deletions(-) >> > >> > diff --git a/refs.c b/refs.c >> > index e7606716d..0272e332c 100644 >> > --- a/refs.c >> > +++ b/refs.c >> > @@ -366,11 +366,11 @@ int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data) >> > >> > const char *prettify_refname(const char *name) >> > { >> > - return name + ( >> > - starts_with(name, "refs/heads/") ? 11 : >> > - starts_with(name, "refs/tags/") ? 10 : >> > - starts_with(name, "refs/remotes/") ? 13 : >> > - 0); >> > + if (skip_prefix(name, "refs/heads/", &name) || >> > + skip_prefix(name, "refs/tags/", &name) || >> > + skip_prefix(name, "refs/remotes/", &name)) >> > + ; /* nothing */ >> > + return name; >> >> Nice, but why add the "if" when it's doing nothing? > > It's short-circuiting in the conditional. I think René meant this: /* just for side effects */ skip_prefix(name, "refs/heads/", &name) || skip_prefix(name, "refs/tags/", &name) || skip_prefix(name, "refs/remotes/", &name); return name; which still short-sircuits, even though I do think it looks strange; "correct but strange".