[03/15] name-rev: use strip_suffix() in get_rev_name()
diff mbox series

Message ID 20190919214712.7348-4-szeder.dev@gmail.com
State New
Headers show
Series
  • name-rev: eliminate recursion
Related show

Commit Message

SZEDER Gábor Sept. 19, 2019, 9:46 p.m. UTC
Use strip_suffix() instead of open-coding it, making the code more
idiomatic.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
 builtin/name-rev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

René Scharfe Sept. 20, 2019, 4:36 p.m. UTC | #1
Am 19.09.19 um 23:46 schrieb SZEDER Gábor:
> Use strip_suffix() instead of open-coding it, making the code more
> idiomatic.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>  builtin/name-rev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index c785fe16ba..d345456656 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -317,11 +317,11 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
>  	if (!n->generation)
>  		return n->tip_name;
>  	else {
> -		int len = strlen(n->tip_name);
> -		if (len > 2 && !strcmp(n->tip_name + len - 2, "^0"))
> -			len -= 2;
> +		size_t len;
> +		strip_suffix(n->tip_name, "^0", &len);
>  		strbuf_reset(buf);
> -		strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation);
> +		strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name,
> +			    n->generation);
>  		return buf->buf;
>  	}
>  }
>

This gets rid of the repeated magic string length constant 2, which is
nice.  But why not go all the way to full strbuf-ness?  It's shorter,
looks less busy, and the extra two copied bytes shouldn't matter in a
measurable way.

	else {
		strbuf_reset(buf);
		strbuf_addstr(buf, n->tip_name);
		strbuf_strip_suffix(buf, "^0");
		strbuf_addf(buf, "~%d", n->generation);
		return buf->buf;
	}
SZEDER Gábor Sept. 20, 2019, 5:10 p.m. UTC | #2
On Fri, Sep 20, 2019 at 06:36:30PM +0200, René Scharfe wrote:
> Am 19.09.19 um 23:46 schrieb SZEDER Gábor:
> > Use strip_suffix() instead of open-coding it, making the code more
> > idiomatic.
> >
> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> > ---
> >  builtin/name-rev.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> > index c785fe16ba..d345456656 100644
> > --- a/builtin/name-rev.c
> > +++ b/builtin/name-rev.c
> > @@ -317,11 +317,11 @@ static const char *get_rev_name(const struct object *o, struct strbuf *buf)
> >  	if (!n->generation)
> >  		return n->tip_name;
> >  	else {
> > -		int len = strlen(n->tip_name);
> > -		if (len > 2 && !strcmp(n->tip_name + len - 2, "^0"))
> > -			len -= 2;
> > +		size_t len;
> > +		strip_suffix(n->tip_name, "^0", &len);
> >  		strbuf_reset(buf);
> > -		strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation);
> > +		strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name,
> > +			    n->generation);
> >  		return buf->buf;
> >  	}
> >  }
> >
> 
> This gets rid of the repeated magic string length constant 2, which is
> nice.  But why not go all the way to full strbuf-ness?  It's shorter,
> looks less busy, and the extra two copied bytes shouldn't matter in a
> measurable way.
> 
> 	else {
> 		strbuf_reset(buf);
> 		strbuf_addstr(buf, n->tip_name);
> 		strbuf_strip_suffix(buf, "^0");
> 		strbuf_addf(buf, "~%d", n->generation);
> 		return buf->buf;
> 	}

Oh, I like this, thanks!

Patch
diff mbox series

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index c785fe16ba..d345456656 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -317,11 +317,11 @@  static const char *get_rev_name(const struct object *o, struct strbuf *buf)
 	if (!n->generation)
 		return n->tip_name;
 	else {
-		int len = strlen(n->tip_name);
-		if (len > 2 && !strcmp(n->tip_name + len - 2, "^0"))
-			len -= 2;
+		size_t len;
+		strip_suffix(n->tip_name, "^0", &len);
 		strbuf_reset(buf);
-		strbuf_addf(buf, "%.*s~%d", len, n->tip_name, n->generation);
+		strbuf_addf(buf, "%.*s~%d", (int) len, n->tip_name,
+			    n->generation);
 		return buf->buf;
 	}
 }