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

[ZheNing Hu <adlternative@gmail.com>]

Junio C Hamano <gitster@pobox.com> 于2021年5月28日周五 下午12:36写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >               struct {
> >                       enum { RAW_BARE, RAW_LENGTH } option;
> >               } raw_data;
> > +             struct {
> > +                     enum { H_BARE, H_LENGTH } option;
> > +             } header;
>
> Raw does not use R_{BARE,LENGTH} and uses raw_data member.  Header
> should follow suit unless there is a compelling reason not to, no?
>
>                 struct {
>                         enum { HEADER_BARE, HEADER_LENGTH } option;
>                 } header_data;
>
> perhaps?
>

OK.


> > @@ -1372,6 +1389,15 @@ static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned
> >                                   &bodypos, &bodylen, &nonsiglen,
> >                                   &sigpos, &siglen);
> >
> > +             if (starts_with(name, "header")) {
> > +                     size_t header_len = subpos - (const char *)buf - 1;
>
> Hmph, is this correct?  I would expect that the "header" part of a
> commit or a tag object excludes the blank line after the header
> fields.  In other words, the "header" would be separated by a blank
> line from the "body", and that separating blank line is not part of
> "header" or "body".
>
> Otherwise, if there is a user of %(header), it needs to be coded to
> ignore the last blank line but has to diagnose it as an error if
> there is a blank line before that.
>

I am a bit confused, Is there any problem with me doing this?

> > +                     size_t header_len = subpos - (const char *)buf - 1;

"header" part starts from "buf" and header_len have minus 1 so that
header part will not touch the blank line. At the same time, "contents"
part starts from subpos, and it also does not touch the blank line.

> While having this may not be wrong, I am not sure who needs it.  Is
> your "cat-file --batch" topic needs this new atom?

Ok, I will remove it from this topic temporarily.

Thanks.
--
ZheNing Hu