Re: [PATCH 1/2] ls-files: adding support for submodules

From: Jeff King <peff@peff.net>
Date: Sep 22, 2016, 04:18 UTC
Message-ID: <20160922041854.7754ujcynhk7mdnh@sigill.intra.peff.net>
In-Reply-To: <xmqqponw6e3x.fsf@gitster.mtv.corp.google.com>

On Wed, Sep 21, 2016 at 04:13:22PM -0700, Junio C Hamano wrote:

Show 9 quoted lines

> Brandon Williams <bmwill@google.com> writes:
> 
> > yes you mentioned this and I meant to change that before sending it out.
> > Looks like it slipped through have slipped through.
> 
> I already fixed it up locally when I sent the reply, but thanks for
> resending (which assures me that your local copy is up-to-date and I
> do not have to worry about having to repeat me in the future, if
> this ever needs further rerolling ;-).

While we are on the subject, the commit message also uses some past tense:

  Allow ls-files to recognize submodules in order to retrieve a list of
  files from a repository's submodules.  This is done by forking off a
  process to recursively call ls-files on all submodules. Also added a
  submodule-prefix command in order to prepend paths to child processes.

The final sentence should be "Also add...".

Since this final bit of logic was sufficiently non-obvious that it only came about in v2, maybe it is worth describing a little more fully:

  Also add a submodule-prefix option, which instructs the child
  processes to prepend the prefix to each path they output. This makes
  the output paths match what is on the filesystem (i.e., as if the
  submodule boundaries were not there at all).

Should this option just be "--prefix", or maybe "--output-prefix"? Submodules are the obvious use case here, but I could see somebody adapting this for other uses (alternatively, if we _do_ want to keep it just as an implementation detail for submodules, we should probably discourage people in the documentation from using it themselves).

-Peff