Re: [PATCH 5/8] [GSOC] ref-filter: teach get_object() return useful value

>
>
> On Sat, Jun 12 2021, ZheNing Hu via GitGitGadget wrote:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > Let `populate_value()`, `get_ref_atom_value()` and
> > `format_ref_array_item()` get the return value of `get_value()`
> > correctly. This can help us later let `cat-file --batch` get the
> > correct error message and return value of `get_value()`.
> >
> > Mentored-by: Christian Couder <christian.couder@gmail.com>
> > Mentored-by: Hariom Verma <hariom18599@gmail.com>
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > ---
> >  ref-filter.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/ref-filter.c b/ref-filter.c
> > index 8868cf98f090..420c0bf9384f 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -1808,7 +1808,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
> >  static int populate_value(struct ref_array_item *ref, struct strbuf *err)
> >  {
> >       struct object *obj;
> > -     int i;
> > +     int i, ret = 0;
>
> The usual style is to not bunch up variables based on type, but only if
> they're related, i.e. we'd do:
>
>     if i, j; /* proceed to use i and j in two for-loops */
>
> But:
>
>     int i; /* for the for-loop */
>     int ret = 0; /* for our return value */
>
> (Without the comments)
>

> >       struct object_info empty = OBJECT_INFO_INIT;
> >
> >       CALLOC_ARRAY(ref->value, used_atom_cnt);
> > @@ -1965,8 +1965,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
> >
> >
> >       oi.oid = ref->objectname;
> > -     if (get_object(ref, 0, &obj, &oi, err))
> > -             return -1;
> > +     if ((ret = get_object(ref, 0, &obj, &oi, err)))
> > +             return ret;
>
> Maybe more personal style, I'd just write this as:
>
>     ret = x();
>     if (!ret)
>         return ret;
>
> Makes it easier to read and balance parens in your head for the common
> case...
>

> > @@ -2585,10 +2588,10 @@ int format_ref_array_item(struct ref_array_item *info,
> >               if (cp < sp)
> >                       append_literal(cp, sp, &state);
> >               pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
> > -             if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||
> > +             if (pos < 0 || (ret = get_ref_atom_value(info, pos, &atomv, error_buf)) ||
> >                   atomv->handler(atomv, &state, error_buf)) {
> >                       pop_stack_element(&state.stack);
>
> ... and use that mental energy on readist stuff like this, which I'd
> just leave as the inline assignment.