Subject: Re: [PATCH 3/8] pathspec: change PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag

From: Brandon Williams <[email-masked]>
Date: Wed, 10 May 2017 09:16:33 -0700

On 05/10, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > It's confusing to have two different 'strip submodule slash' flags which
> > do subtly different things.  Both
> > PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE and
> > PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP will accomplish the same task of
> > striping a slash from a pathspec which matches a submodule entry in the
> > index.  The only difference is that
> > PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE will perform additional checks
> > and die if a pathspec has a leading path component which corresponds to
> > a submodule.  This additional functionality should be split out into its
> > own flag.
> >
> > To this end, rename the PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE flag to
> > PATHSPEC_SUBMODULE_LEADING_PATH and change its behavior to only die if a
> > path descends into a submodule.  In addition add the
> > PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to callers which relied on the
> > old slash stripping functionality.
> 
> "PATHSPEC_SUBMODULE_LEADING_PATH" feels like an unfinished sentence
> to me.  Do I understand your description correctly if I say it is
> about "checking" the leading path to see if it overlaps with a
> submodule path?  IOW, I am wondering if the name should have the
> word CHECK somewhere in it.
> 

You're probably right, the name has something left to be desired.  I
chose it simply because it conforms to another flag
"PATHSPEC_SYMLINK_LEADING_PATH" which dies if there is a symlink in the
leading path.

And you are correct, it checks if the leading path overlaps with a
submodule path.  If it strictly matches a submodule path that's alright
though.  The point of this flag is to disallow paths which descend into
submodules.  One such use case is to prevent a user from trying to 'git
add' a file from a submodule.

-- 
Brandon Williams