Subject: Re: [PATCH 1/4 v4] submodules: make submodule-prefix option

From: Brandon Williams <[email-masked]>
Date: Tue, 27 Sep 2016 13:29:25 -0700

On 09/27, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > +--submodule-prefix=<path>::
> > +	Set a prefix which gives submodules context about the superproject that
> > +	invoked it.  Only allowed for commands which support submodules.
> 
> This, and also the message in die(), uses a phrase "support
> submodules", but it is unclear what it exactly means to the end
> users and readers.
> 
> A "ls-files" that is recursively run as an implementation detail of
> the "grep --recurse-submodules" would be taught to support this
> option with this series.  Who is supporting submodules in that
> context?
> 
> I'd imagine (close to) 100% of the people would say it is "grep"
> that is supporting submodules, not "ls-files", but what this
> paragraph and die() message want to express by the phrase "support
> submodules" is the fact that "ls-files" knows how to react to
> "--submodule-prefix" option.
> 
> I'd suggest not to worry too much about this phrasing at this point,
> until we figure out exactly how we want to present these to end
> users.  For now, perhaps drop the second sentence and replace it
> with "The end-users are not expected to use this option" or
> something like that?

K can do.  The intention is that each command has to do whatever
internal rework needed so that it understands how to interact with
subomodules (be that calling another command or internally supporting
it).

> > +		die("%s doesn't support submodules", p->cmd);
> 
> s/submodules/submodule-prefix/ at least.

So should the #define be SUPPORT_SUBMODULE_PREFIX instead?  That may be
too narrow minded and not looking toward future submodule options
support but I'm not sure.

-- 
Brandon Williams