Re: [PATCH v16 00/14] port tag.c to use ref-filter APIs

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Karthik Nayak <karthik.188@gmail.com> writes:

> @@ -705,9 +719,12 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
>  			v->s = xstrdup(subpos);
>  		else if (skip_prefix(name, "contents:lines=", &valp)) {
>  			struct strbuf s = STRBUF_INIT;
> +			const char *contents_end = bodylen + bodypos - siglen;
> +
>  			if (strtoul_ui(valp, 10, &v->u.contents.lines))
> -				die(_("positive width expected contents:lines=%s"), valp);
> -			append_lines(&s, subpos, sublen + bodylen - siglen, v->u.contents.lines);
> +				die(_("positive value expected contents:lines=%s"), valp);
> +			/*  Size is the length of the message after removing the signature */

Nit: double-space after /*.

> -test_expect_success 'alignment with format quote' '
> -	cat >expect <<-EOF &&
> -	refname is ${sq}           ${sq}\\${sq}${sq}master${sq}\\${sq}${sq}           ${sq}|
> -	refname is ${sq}            ${sq}\\${sq}${sq}side${sq}\\${sq}${sq}            ${sq}|
> -	refname is ${sq}          ${sq}\\${sq}${sq}odd/spot${sq}\\${sq}${sq}          ${sq}|
> -	refname is ${sq}         ${sq}\\${sq}${sq}double-tag${sq}\\${sq}${sq}         ${sq}|
> -	refname is ${sq}            ${sq}\\${sq}${sq}four${sq}\\${sq}${sq}            ${sq}|
> -	refname is ${sq}            ${sq}\\${sq}${sq}one${sq}\\${sq}${sq}             ${sq}|
> -	refname is ${sq}         ${sq}\\${sq}${sq}signed-tag${sq}\\${sq}${sq}         ${sq}|
> -	refname is ${sq}           ${sq}\\${sq}${sq}three${sq}\\${sq}${sq}            ${sq}|
> -	refname is ${sq}            ${sq}\\${sq}${sq}two${sq}\\${sq}${sq}             ${sq}|
> +test_expect_success 'alignment with format quote' "
> +	cat >expect <<-\EOF &&
> +	|'       master| A U Thor       '|
> +	|'        side| A U Thor        '|
> +	|'      odd/spot| A U Thor      '|
> +	|'         double-tag|          '|
> +	|'        four| A U Thor        '|
> +	|'        one| A U Thor         '|
> +	|'         signed-tag|          '|
> +	|'       three| A U Thor        '|
> +	|'        two| A U Thor         '|
>  	EOF
> -	git for-each-ref --shell --format="refname is %(align:30,middle)${sq}%(refname:short)${sq}%(end)|" >actual &&
> +	git for-each-ref --shell --format='|%(align:30,middle)%(refname:short)| %(authorname)%(end)|' >actual &&
>  	test_cmp expect actual
> -'
> +"

The new test is indeed easier to read, but you're not testing anymore
the consequence of a single-quote within the aligned string.

An implementation that just adds single-quotes around the string without
turning ' into '\'' would be broken but still pass the test.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/