git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ls-tree: add --full-tree option
@ 2008-12-26  0:54 Junio C Hamano
  2008-12-26  8:16 ` Johannes Sixt
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2008-12-26  0:54 UTC (permalink / raw)
  To: git; +Cc: Deskin Miller

The established behaviour of "git ls-tree $commit" run from a subdirectory
"sub/dir" in a work tree is to limit the output to the paths in the
subdirectory, and strip off the leading "sub/dir" from the output, since
3c5e846 (ls-tree: major rewrite to do pathspec, 2005-11-26).

This was a "usability" feature made back in the days when the line between
Porcelain and plumbing was blurry, and in retrospect, it probably was
misguided.  The behaviour may be what the end user would expect when the
command is run interactively from a subdirectory, but it also means that a
scripted Porcelain that wants to use the command to list the full contents
of a tree object has to do cd_to_toplevel (and save the output from
"rev-parse --show-prefix" before doing so, so that it can be used as a
pathspec if it wants to limit its operation to the original subdirectory
in other commands).

This new option makes the command operate on the full tree object,
regardless of where in the work tree it is run from.  It also implies the
behaviour that is triggered by the existing --full-name option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * There was a discussion on #git between Ilari and deskin this morning;
   hopefully I captured what they wanted correctly.

   I do not care too deeply with this change myself, though...

 builtin-ls-tree.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index cb61717..c386aa5 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -156,6 +156,11 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 				chomp_prefix = 0;
 				break;
 			}
+			if (!strcmp(argv[1]+2, "full-tree")) {
+				ls_tree_prefix = prefix = NULL;
+				chomp_prefix = 0;
+				break;
+			}
 			if (!prefixcmp(argv[1]+2, "abbrev=")) {
 				abbrev = strtoul(argv[1]+9, NULL, 10);
 				if (abbrev && abbrev < MINIMUM_ABBREV)
-- 
1.6.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ls-tree: add --full-tree option
  2008-12-26  0:54 [PATCH] ls-tree: add --full-tree option Junio C Hamano
@ 2008-12-26  8:16 ` Johannes Sixt
  2008-12-26  8:55   ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2008-12-26  8:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Deskin Miller

On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
> This new option makes the command operate on the full tree object,
> regardless of where in the work tree it is run from.  It also implies the
> behaviour that is triggered by the existing --full-name option.

What's wrong with using 'git ls-tree ${rev}:'?

Except that it does not work and still silently inserts the subdirectory, 
which I consider a serious bug...

-- Hannes

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ls-tree: add --full-tree option
  2008-12-26  8:16 ` Johannes Sixt
@ 2008-12-26  8:55   ` Junio C Hamano
  2008-12-26 10:31     ` demerphq
  2008-12-26 19:49     ` Johannes Sixt
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2008-12-26  8:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Deskin Miller

Johannes Sixt <j6t@kdbg.org> writes:

> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>> This new option makes the command operate on the full tree object,
>> regardless of where in the work tree it is run from.  It also implies the
>> behaviour that is triggered by the existing --full-name option.
>
> What's wrong with using 'git ls-tree ${rev}:'?
>
> Except that it does not work...

Hmph... you seem to be describing the exact issue they discussed on #git,
which triggered the patch in the message you are responding to.  I am not
sure what to say to your "What's wrong with...".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ls-tree: add --full-tree option
  2008-12-26  8:55   ` Junio C Hamano
@ 2008-12-26 10:31     ` demerphq
  2008-12-26 19:49     ` Johannes Sixt
  1 sibling, 0 replies; 6+ messages in thread
From: demerphq @ 2008-12-26 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, git, Deskin Miller

2008/12/26 Junio C Hamano <gitster@pobox.com>:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>>> This new option makes the command operate on the full tree object,
>>> regardless of where in the work tree it is run from.  It also implies the
>>> behaviour that is triggered by the existing --full-name option.
>>
>> What's wrong with using 'git ls-tree ${rev}:'?
>>
>> Except that it does not work...
>
> Hmph... you seem to be describing the exact issue they discussed on #git,
> which triggered the patch in the message you are responding to.  I am not
> sure what to say to your "What's wrong with...".

AFAIK I started that discussion, and during it we also mentioned that
it would be very useful to be able to use git ls-files in non
recursive mode. As the advantage of using ls-tree, prior to your fix,
was that it is the only way to get a list of files for the current
working directory and only the current working directory.

Since you have fixed that bug what command does one us to get a list
of tracked files for the current working directory without recursing
into subdirectories? Can we have a switch on ls-files that disables
recursion? Please?

Cheers,
Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ls-tree: add --full-tree option
  2008-12-26  8:55   ` Junio C Hamano
  2008-12-26 10:31     ` demerphq
@ 2008-12-26 19:49     ` Johannes Sixt
  2008-12-26 21:38       ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2008-12-26 19:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Deskin Miller

Junio C Hamano schrieb:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> On Freitag, 26. Dezember 2008, Junio C Hamano wrote:
>>> This new option makes the command operate on the full tree object,
>>> regardless of where in the work tree it is run from.  It also implies the
>>> behaviour that is triggered by the existing --full-name option.
>> What's wrong with using 'git ls-tree ${rev}:'?
>>
>> Except that it does not work...
> 
> Hmph... you seem to be describing the exact issue they discussed on #git,
> which triggered the patch in the message you are responding to.  I am not
> sure what to say to your "What's wrong with...".

I'm saying that if a script has to be fixed to use --full-tree, then it 
can be fixed just as well by appending the colon to the ${rev}.

Not even porcelain 'git show ${rev}:' "is allowed" to insert the prefix. 
Dscho has argued the case passionately in the past:

http://thread.gmane.org/gmane.comp.version-control.git/68786/focus=68846

Therefore, plumbing 'ls-tree ${rev}:' shouldn't do that, either.

OTOH, you had yourself argued somewhat in favor of the current ls-tree 
behavior:

http://thread.gmane.org/gmane.comp.version-control.git/46232/focus=46400

I'm personally more in line with Dscho, and think that the current 
ls-tree behavior is a terrible bug.

-- Hannes

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ls-tree: add --full-tree option
  2008-12-26 19:49     ` Johannes Sixt
@ 2008-12-26 21:38       ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2008-12-26 21:38 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Deskin Miller

Johannes Sixt <j6t@kdbg.org> writes:

> I'm saying that if a script has to be fixed to use --full-tree, then
> it can be fixed just as well by appending the colon to the ${rev}.

I do not agree with your "just as well"; an existing script that feeds a
tree object to the plumbing would be broken by such a change.

But I think perhaps you were responding to the first paragraph of the
commit log message that you omitted from your quote?

JC> The established behaviour of "git ls-tree $commit" run from a
JC> subdirectory "sub/dir" in a work tree is to...

If that is the case, I understand what you meant.

The patch is about the behaviour of the command for not just $commit but
any $tree_ish, so "git ls-tree ${commit}:" shares the exact same issue
(i.e. historical background that forbids us from changing the behaviour
without an explicit option, and that --full-tree can be a way to help new
scripts without breaking existing scripts' expectations).

I've updated the commit log message with s/$commit/$tree_ish/;

> OTOH, you had yourself argued somewhat in favor of the current ls-tree
> behavior:
>
> http://thread.gmane.org/gmane.comp.version-control.git/46232/focus=46400

That's not really "somewhat in favor".  I can be (and am more often than
not) sympathetic without agreeing to the end result. My sympathy extends
from "I can sort-of-kind-of imagine that it may hurt, and even though I do
not think your approach is a way to properly address it at all, I'd agree
it might be nice to have some solution to the issue" to "I do not think it
is feasible to change this anymore, but I wish we could, too."

In any case, the quoted message was from May 2007, way before v1.6.0 when
we learned the hard way that people do not want any change.

I really hate "take it or leave it", especially when it is not my itch to
scratch, but in this case, I think I've spent enough time making myself
clear that I think (1) that the current behaviour is a result of misguided
attempt for interactive user expectation, which shouldn't have made to the
plumbing, (2) that we cannot change the default behaviour now even (1) is
true; and (3) that the only possible approach to help new scripts would be
a new option.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-12-26 21:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-26  0:54 [PATCH] ls-tree: add --full-tree option Junio C Hamano
2008-12-26  8:16 ` Johannes Sixt
2008-12-26  8:55   ` Junio C Hamano
2008-12-26 10:31     ` demerphq
2008-12-26 19:49     ` Johannes Sixt
2008-12-26 21:38       ` Junio C Hamano

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 NNTP newsgroup(s).