On Wed, May 15, 2024 at 6:46 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Christian Couder <christian.couder@xxxxxxxxx> writes:
> > static int option_parse_missing_action(const struct option *opt UNUSED,
> > const char *arg, int unset)
> > {
> > + int res;
> > +
> > assert(arg);
> > assert(!unset);
> >
> > + res = parse_missing_action_value(arg);
> > + if (res < 0 || (res != MA_ERROR &&
> > + res != MA_ALLOW_ANY &&
> > + res != MA_ALLOW_PROMISOR))
> > + die(_("invalid value for '%s': '%s'"), "--missing", arg);
>
> What is our expectation for how <missing.h> API would evolve over
> time? I think it is a given that it will always be a superset of
> the need of rev-list and the need of pack-objects, but if we were
> to add a new value of MA_FOO, do we expect that all of the new ones
> are not handled by pack-objects, Some but not all? Or none of the
> new ones are handled by pack-objects?
I don't think the <missing.h> API would evolve much over time. At
least I don't think we have plans to make it evolve. Perhaps other
options similar to MA_PRINT could be added though, maybe MA_TRACE or
MA_LOG. Maybe such a new option could be handled by pack-object, maybe
not. I think for now it's better to be flexible and not guess too
much.
> Regardless of the answer to that question, I think a simple helper
> is warranted here, which will also help the [3/3] which adds exactly
> the same code to upload-pack.c:upload_pack_config(), so that the
> callers can do
>
> res = parse_missing_action_value_for_packing(arg);
> if (res < 0)
> die(_("invalid value for '%s': '%s'"), "--missing", arg);
Ok I implemented this in the v3 I just sent.
> > + if (res != MA_ERROR)
> > fetch_if_missing = 0;
> > + arg_missing_action = res;
> > + fn_show_object = show_object_fn_from_action(arg_missing_action);
> >
> > - die(_("invalid value for '%s': '%s'"), "--missing", arg);
> > return 0;
> > }
>
> Hmph, wouldn't a small array of show_object_fn suffice, making the
> whole thing more like:
>
> static show_object_fn const fn[] = {
> [MA_ERROR] = show_object,
> [MA_ALLOW_ANY] = show_object__ma_allow_any,
> [MA_ALLOW_PROMISOR] = show_object__ma_allow_promisor,
> };
>
> res = parse_missing_action_value_for_packing(arg);
> if (res < 0 || ARRAY_SIZE[fn] <= res)
> die(_("invalid value for '%s': '%s'"), "--missing", arg);
> fn_show_object = fn[res];
> return 0;
>
> without the need for show_object_fn_from_action() helper function?
Ok with the small array. I implemented it in v3 too.
Thanks for the suggestions.
