Re: [PATCH v3 02/10] submodule status: use submodule--helper is-active

> On 03/14, Junio C Hamano wrote:
> > Brandon Williams <bmwill@google.com> writes:
> > 
> > > Signed-off-by: Brandon Williams <bmwill@google.com>
> > > ---
> > >  git-submodule.sh | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/git-submodule.sh b/git-submodule.sh
> > > index 136e26a2c..ab233712d 100755
> > > --- a/git-submodule.sh
> > > +++ b/git-submodule.sh
> > > @@ -1010,14 +1010,13 @@ cmd_status()
> > >  	do
> > >  		die_if_unmatched "$mode" "$sha1"
> > >  		name=$(git submodule--helper name "$sm_path") || exit
> > > -		url=$(git config submodule."$name".url)
> > >  		displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix")
> > >  		if test "$stage" = U
> > >  		then
> > >  			say "U$sha1 $displaypath"
> > >  			continue
> > >  		fi
> > > -		if test -z "$url" ||
> > > +		if ! git submodule--helper is-active "$sm_path" ||
> > >  		{
> > >  			! test -d "$sm_path"/.git &&
> > >  			! test -f "$sm_path"/.git
> > 
> > The $name is no longer used after this step in cmd_status function, 
> > as the sole purpose of learning the name from the path was so that
> > we can ask if the submodule has .URL defined and the query is done
> > by name, not path.
> 
> Thanks, I'll remove computing the name.

> 
> > 
> > This actually raises a question.  
> > 
> > Shouldn't "submodule--helper is-active" ask about submodule while
> > identifying the submodule in question by name?  Or do all (or most
> > of) the callers start from path and ask is-active on them so that it
> > is handier to let them ask by path?
> 
> As you can Stefan already discussed the path is a much easier handle to
> work with simply because that is what is more readily available as the
> name is an internal detail.
> 
> -- 
> Brandon Williams