git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Brandon Williams <bmwill@google.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] ls-files: adding support for submodules
Date: Thu, 22 Sep 2016 11:13:13 -0700	[thread overview]
Message-ID: <xmqq7fa36bwm.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160922041854.7754ujcynhk7mdnh@sigill.intra.peff.net> (Jeff King's message of "Thu, 22 Sep 2016 00:18:54 -0400")

Jeff King <peff@peff.net> writes:

> 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).

I agree that this is not specific to submodules; this is closely
related to what we internally call "prefix", but is different.

In any case, I would strongly recommend against exposing this (or
anything for that matter) "--prefix" to the end-user, especially
because this feature is likely to be applicable to many subcommands,
and some subcommands would want different sort of prefixing made to
different things.  Think of "git diff" that has a way to customize
the "a/" and "b/" part in "git --diff a/$path b/$path", and has
learned another way to prepend an additional prefix to every line of
output via "--line-prefix".  We want to give reasonably specific
names to things so that readers can tell what it affects from the
name of an option.

What we internally call "prefix" and "--submodule-prefix" is closely
related in that they both interact with pathspecs.  "prefix" gets
prepended to elements of an end-user supplied pathspec before a
full-path-in-the-repository (i.e. a path in the index and a path
relative from the top of the working tree) is matched against them.
This new thing on the other hand allows the leading part of pathspec
elements to be above the full-path-in-the-repository.  For example,
my primary Git working area is at ~/w/git.git and I could say

    $ cd ~
    $ git -C w/git.git/ ls-files \
        --submodule-prefix=w/git.git -- 'w/git.git/D\*' |
      xargs ls -1 -l

The command starts (eh, at least "pretends to start") in the top of
my working tree, which means that the "prefix" is NULL.  But the
shell that spawns the "git" command is still sitting in my home
directory, and viewed from there, the Documentation subdirectory is
at w/git.git/Documentation/, which is what I gave the command as the
sole element of the pathspec.  From "ls-files"'s point of view, each
path it discovers in the index (and in the working tree) is first
prefixed with --submodule-prefix before it gets matched against this
pathspec and the result is shown with this prefix, so that the
output gets relative to the calling shell and xargs would find them
at expected places.  The --submodule-prefix acts to cancel out the
fact that the caller sits a few levels above the working tree and
gave a pathspec with elements relative to that higher level in the
directory hierarchy.

Obviously, the above example is *not* using submodules at all, and
demonstrates that the mechanism does not have to be used solely to
implement recurse-into-submodules behaviour, so in that sense, the
name of the option is not quite accurate.  It however is a good
demonstration why it is *not* "output prefix".  It makes me wonder
if pathspec-prefix is a better name, but that may give an incorrect
impression that this would be used only for matching and would not
affect the output.  Do we need separate options to specify what gets
prefixed to the in-repository path before paths are matched against
a pathspec (i.e. "--pathspec-prefix") and what gets prefixed to the
resulting paths in the output (i.e. "--output-path-prefix")?

A few further random thoughts.

 * As Stefan alluded to (much) earlier, it might be a better idea
   to have these 'prefix' as the global option to "git" potty, not
   to each subcommand that happens to support them;

 * It is unclear how this should interact with commands that are run
   in a subdirectory of the working tree.  E.g. what should the
   prefix and the pathspec look like if the command in the above
   example is started in w/git.git/Documentation subdirectory, i.e.

    $ cd ~
    $ git -C w/git.git/Documentation ls-files \
        --submodule-prefix=??????? -- '???????' |
      xargs ls -1 -l

   Should we error out if we are not at the top of the working tree
   when --submodule-prefix is given?

 * It is further unclear how this should interact with commands that
   can give "relative" paths output if we allowed them to start from
   a subdirectory of the working tree.  E.g. "git grep" gives its
   output relative to where it was started from:

    $ cd ~/w/git.git/Documentation
    $ git grep -c 'ERROR.*frotz' -- '*.txt'
    git-checkout.txt:1

   How should it interact with the --submodule-prefix option and
   what value should the caller give to the option and how should
   the caller adjust the pathspec if it wants to get paths relative
   to my home directory?  I do not think the following is correct,
   but I am not sure what the correct values of them should be:

    $ cd ~
    $ git -C w/git.git/Documentation \
      --submodule-prefix=w/git.git/ \
      grep -c 'ERROR.*frotz' -- 'w/git.git/Documentation/*.txt'

   This becomes a non-issue if we forbid use of this new prefix when
   "git" is not started at the top of the working tree.


  parent reply	other threads:[~2016-09-22 18:14 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-21 22:04 [PATCH 1/2] ls-files: adding support for submodules Brandon Williams
2016-09-21 22:04 ` [PATCH 2/2] ls-files: add pathspec matching " Brandon Williams
2016-09-21 22:53   ` Junio C Hamano
2016-09-21 23:23     ` Brandon Williams
2016-09-21 23:28       ` [PATCH 2/2 v2] " Brandon Williams
2016-09-23 18:48         ` Junio C Hamano
2016-09-23 19:20         ` Junio C Hamano
2016-09-23 20:49           ` Brandon Williams
2016-09-21 22:08 ` [PATCH 1/2] ls-files: adding support " Brandon Williams
2016-09-21 22:28   ` Junio C Hamano
2016-09-21 22:38     ` Brandon Williams
2016-09-21 22:42       ` [PATCH 1/2] ls-files: optionally recurse into submodules Brandon Williams
2016-09-22  6:20         ` Jeff King
2016-09-23 23:31           ` Brandon Williams
2016-09-21 23:13       ` [PATCH 1/2] ls-files: adding support for submodules Junio C Hamano
2016-09-22  4:18         ` Jeff King
2016-09-22 16:04           ` Stefan Beller
2016-09-22 18:13           ` Junio C Hamano [this message]
2016-09-23  3:41             ` Jeff King
2016-09-23  5:47               ` Stefan Beller
2016-09-23  6:06                 ` Jeff King
2016-09-23 16:16                   ` Brandon Williams
2016-09-23 16:34                     ` Stefan Beller
2016-09-25 11:03                       ` Nazri Ramliy
2016-09-27 21:38             ` Junio C Hamano
2016-09-27 21:48               ` Brandon Williams
2016-09-27 22:01                 ` Junio C Hamano
2016-09-27 22:09                   ` Brandon Williams
2016-09-27 22:23                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7fa36bwm.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).