Re: [PATCH v2 21/25] rev-list: add --index-objects option

2014-10-16 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 There is currently no easy way to ask the revision traversal
 machinery to include objects reachable from the index (e.g.,
 blobs and trees that have not yet been committed). This
 patch adds an option to do so.

 Signed-off-by: Jeff King p...@peff.net
 ---
 I was tempted to call this just --index, because I could not think of
 what else --index would mean in the context of rev-list. But I also
 worried about weird interactions with other commands that take revision
 arguments. And since this is mostly for internal use anyway, I figured
 the more verbose name is not too bad. I could be convinced otherwise,
 though.

I agree that --index is a bad name as it usually is used in a
particular context: the command can work on various combination of
working tree and the index, and I am asking it to work on both
(e.g. apply --index as opposed to apply --cached).

 diff --git a/Documentation/rev-list-options.txt 
 b/Documentation/rev-list-options.txt
 index 4cf94c6..03ab343 100644
 --- a/Documentation/rev-list-options.txt
 +++ b/Documentation/rev-list-options.txt
 @@ -172,6 +172,13 @@ explicitly.
   Pretend as if all objects mentioned by reflogs are listed on the
   command line as `commit`.
  
 +--index-objects::

This risks index getting misunderstood as a verb, e.g. please
enumerate the objects and assign labels to later refer to them,
doesn't it?

--indexed-objects (short for --show-objects-in-the-index) or
something?

 + Pretend as if all objects used by the index (any blobs, and any
 + trees which are mentioned by the index's cache-tree extension)
 + ad listed on the command line. Note that you probably want to

s/ad/are/, probably?

 + use `--objects`, too, as there are by definition no commits in
 + the index.

For gitlinks/submodules, the index records names of the commit
objects, they are not listed, and that is the right behaviour, but
this description invites some confusion.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 21/25] rev-list: add --index-objects option

2014-10-16 Thread Jeff King
On Thu, Oct 16, 2014 at 11:41:35AM -0700, Junio C Hamano wrote:

 I agree that --index is a bad name as it usually is used in a
 particular context: the command can work on various combination of
 working tree and the index, and I am asking it to work on both
 (e.g. apply --index as opposed to apply --cached).

Thanks. I wasn't sure if I was just being paranoid or not, but now there
are at least two of us.

  +--index-objects::
 
 This risks index getting misunderstood as a verb, e.g. please
 enumerate the objects and assign labels to later refer to them,
 doesn't it?
 
 --indexed-objects (short for --show-objects-in-the-index) or
 something?

That sounds reasonable. We could technically do `--indexed` as that is
different from `--index`, but maybe they are still confusingly close.

  +   Pretend as if all objects used by the index (any blobs, and any
  +   trees which are mentioned by the index's cache-tree extension)
  +   ad listed on the command line. Note that you probably want to
 
 s/ad/are/, probably?

Yeah, sorry, vi cruft I think (at least I didn't accidentally insert
C-a C-k ;) ).

  +   use `--objects`, too, as there are by definition no commits in
  +   the index.
 
 For gitlinks/submodules, the index records names of the commit
 objects, they are not listed, and that is the right behaviour, but
 this description invites some confusion.

Good point. How about this:

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 03ab343..3301fde 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -172,12 +172,10 @@ explicitly.
Pretend as if all objects mentioned by reflogs are listed on the
command line as `commit`.
 
---index-objects::
-   Pretend as if all objects used by the index (any blobs, and any
-   trees which are mentioned by the index's cache-tree extension)
-   ad listed on the command line. Note that you probably want to
-   use `--objects`, too, as there are by definition no commits in
-   the index.
+--indexed-objects::
+   Pretend as if all trees and blobs used by the index are listed
+   on the command line.  Note that you probably want to use
+   `--objects`, too.
 
 --ignore-missing::
Upon seeing an invalid object name in the input, pretend as if

I was tempted to not document this at all (nor to add documentation for
--reflog), as I think these are really only going to be used internally.
But it's nice to have documentation even for this internal stuff (if
anything, we should probably be making sure they are just limited to
rev-list plumbing, and not included in the log manpage).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 21/25] rev-list: add --index-objects option

2014-10-16 Thread Jeff King
On Thu, Oct 16, 2014 at 08:12:31PM -0400, Jeff King wrote:

  --indexed-objects (short for --show-objects-in-the-index) or
  something?
 
 That sounds reasonable. We could technically do `--indexed` as that is
 different from `--index`, but maybe they are still confusingly close.

Here's a re-roll of the final 5 patches of the series with the updated
name (`--indexed-objects`). The name change cascades from patch 22 to
patches 23 and 25 (and I renamed the matching pack-objects option as
well). 24 and 26 are unchanged, but I figured it is easier on you to
replace the whole block of patches at once.

  [22/26]: rev-list: add --indexed-objects option
  [23/26]: reachable: use revision machinery's --indexed-objects code
  [24/26]: pack-objects: use argv_array
  [25/26]: repack: pack objects mentioned by the index
  [26/26]: pack-objects: double-check options before discarding objects
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html