On Thu, Nov 15, 2018 at 11:54 AM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
>
> > +/*
> > + * Initialize 'out' based on the provided submodule path.
> > + *
> > + * Unlike repo_submodule_init, this tolerates submodules not present
> > + * in .gitmodules. This function exists only to preserve historical behavior,
> > + *
> > + * Returns 0 on success, -1 when the submodule is not present.
> > */
> > -static void show_submodule_header(struct diff_options *o, const char *path,
> > +static struct repository *open_submodule(const char *path)
>
> The function documentation needs to be reworded - there's no "out", and
> the return value is now a possibly NULL pointer to struct repository.
Noted.
>
> > +{
> > + struct strbuf sb = STRBUF_INIT;
> > + struct repository *out = xmalloc(sizeof(*out));
> > +
> > + if (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {
> > + strbuf_release(&sb);
> > + free(out);
> > + return NULL;
> > + }
> > +
> > + out->submodule_prefix = xstrdup(path);
>
> I've discussed this submodule_prefix line before [1] - do we really need
> this? Tests pass even if I remove this line.
We might not need it yet as the tests indicate, but it's the right thing to do:
/*
* Path from the root of the top-level superproject down to this
* repository. This is only non-NULL if the repository is initialized
* as a submodule of another repository.
*/
We're not (yet) using this string in our error reporting, but
I anticipate that we'll do eventually.
> Other than that, this patch looks good.
Thanks,
Stefan
