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

[Junio C Hamano <gitster@pobox.com>]

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  struct atom_value {
>  	const char *s;
> +	size_t s_size;
>  	int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,
>  		       struct strbuf *err);
>  	uintmax_t value; /* used for sorting when not FIELD_STR */
>  	struct used_atom *atom;
>  };
>  
> +#define ATOM_VALUE_INIT { \
> +	.s_size = -1 \
> +}

This will get turned into size_t that is unsigned, and comparisons
like this one and ...

>  	case QUOTE_NONE:
> -		strbuf_addstr(s, str);
> +		if (len != -1)
> +			strbuf_add(s, str, len);
> +		else
> +			strbuf_addstr(s, str);

this one ...

> +		if (v->s_size != -1)
> +			strbuf_add(&state->stack->output, v->s, v->s_size);
> +		else
> +			strbuf_addstr(&state->stack->output, v->s);
>  	return 0;
>  }

may work as expected, but it makes readers wonder if there is
significance to negative values other than -1 (in other words, what
does it mean if v->s_size == -2, for example?).

It may make sense to 

 * Turn atom_value.s_size field into ssize_t instead of size_t

 * Rewrite (v->s_size != -1) comparison to (v->s_size < 0)


Optionally we could introduce #define ATOM_SIZE_UNSPECIFIED (-1) and
use it to initialize .s_size in ATOM_VALUE_INIT, but if we are just
going to test "is it negative, then it is not given", then it probably
is not needed.

Other changes in the whole series relative to what has been queued
looked all sensible to me.

THanks.