Re: [PATCH 2/5] [GSOC] ref-filter: add %(raw) atom
- From
ZheNing Hu <adlternative@gmail.com>
- Date
- Jul 22, 2021, 08:31 UTC
- Message-ID
- <CAOLTT8TP4z1vR8i9DX09+oBiPVgaF1isTgD2ZQ4nfoBJwTd9_w@mail.gmail.com>
- In-Reply-To
- <877dhivkei.fsf@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2021年7月22日周四 下午4:10写道:
Show 25 quoted lines
>
>
> On Thu, Jul 22 2021, ZheNing Hu via GitGitGadget wrote:
>
> > [..] add a new member in
> > `struct atom_value`: `s_size`, which can record raw object size, it
> > can help us add raw object data to the buffer or compare two buffers
> > which contain raw object data.
>
> I didn't look in detail, just one nit/comment, no need to re-roll for this:
>
>
> > 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_S_SIZE_INIT (-1)
> > +
>
> This and then later doing (this appears to be the only initialization?):Yes, "s_size = -1” is the only initialization.
Show 23 quoted lines
>
> > if (format->need_color_reset_at_eol) {
> > struct atom_value resetv;
> > + resetv.s_size = ATOM_VALUE_S_SIZE_INIT;
> > resetv.s = GIT_COLOR_RESET;
> > if (append_atom(&resetv, &state, error_buf)) {
> > pop_stack_element(&state.stack);
>
> Is a rather unusual pattern, more typical would be:
>
> struct atom_value { .... }
> #define ATOM_VALUE_INIT { \
> .s_size = -1, \
> }
>
> Then:
>
> struct atom_value resetv = ATOM_VALUE_INIT;
>
> You could keep that "#define ATOM_VALUE_S_SIZE_INIT (-1)" to check the
> init value, personally I think it's just fine to hardcode the -1 literal
> and do "size < 0" checks, which we us as convention in a lot of other
> places for "not initialized".Maybe you are right, hard-coding "-1" is ok.
Thanks. -- ZheNing Hu