Re: [PATCH 12/21] patch-ids.c: remove implicit dependency on the_index

From: Stefan Beller <sbeller@google.com>
Date: Aug 27, 2018, 19:12 UTC
Message-ID: <CAGZ79kb=gcmEDGmTS-j-veFj8BgKup0g96i0Gm1P+j8eyX8_mQ@mail.gmail.com>
In-Reply-To: <20180826100314.5137-13-pclouds@gmail.com>

Show 6 quoted lines

> -int init_patch_ids(struct patch_ids *ids)
> +int init_patch_ids(struct patch_ids *ids, struct repository *repo)
>  {
>         memset(ids, 0, sizeof(*ids));
> -       diff_setup(&ids->diffopts, the_repository);
> +       diff_setup(&ids->diffopts, repo);

Just realized when looking at this diff, though it applies to other patches as well. (and reading Documentation/technical/api-diff.txt confirms my thinking IMHO)

What makes the repository argument any special compared to the rest of the diff options?

So I would expect the setup to look like

    memset(ids, 0, sizeof(*ids));
    ids->diffopts->repo = the_repository;
    diff_setup(&ids->diffopts);

here and in diff_setup, we'd have

  if (!options->repo)
    options->repo = the_repository;

or even put the_repository into default_diff_options, but then I wonder how this deals with no-repo invocations (git diff --no-index examples for bug reports)