[PATCH] git show <tree>: show mode and hash, and handle -r

[PATCH] git show <tree>: show mode and hash, and handle -r

[Johannes Schindelin]

Now, git show <tree> shows some more information, and with the -r option,
it recurses into subdirectories.

Requested by Scott Chacon.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	The only quirk is the command line parsing for -r: we cannot use 
	DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched 
	on not only by cmd_log_init(), but implicitly by 
	diff_setup_done(), because FORMAT_PATCH is selected by git-show.

 builtin-log.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 3796dda..b00d353 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -297,8 +297,12 @@ static int show_tree_object(const unsigned char *sha1,
 		const char *base, int baselen,
 		const char *pathname, unsigned mode, int stage, void *context)
 {
-	printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
-	return 0;
+	int *recursive = context;
+	printf("%06o %s %.*s%s%s\n", mode,
+			find_unique_abbrev(sha1, DEFAULT_ABBREV),
+			baselen, base,
+			pathname, S_ISDIR(mode) ? "/" : "");
+	return *recursive ? READ_TREE_RECURSIVE : 0;
 }
 
 int cmd_show(int argc, const char **argv, const char *prefix)
@@ -306,6 +310,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct object_array_entry *objects;
 	int i, count, ret = 0;
+	int tree_recursive = 0;
 
 	git_config(git_log_config, NULL);
 
@@ -319,6 +324,11 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	rev.always_show_header = 1;
 	rev.ignore_merges = 0;
 	rev.no_walk = 1;
+	for (i = 1; i < argc && strcmp(argv[i], "--"); i++)
+		if (!strcmp(argv[i], "-r")) {
+			tree_recursive = 1;
+			break;
+		}
 	cmd_log_init(argc, argv, prefix, &rev);
 
 	count = rev.pending.nr;
@@ -348,7 +358,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 					name,
 					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
-					show_tree_object, NULL);
+					show_tree_object, &tree_recursive);
 			break;
 		case OBJ_COMMIT:
 			rev.pending.nr = rev.pending.alloc = 0;
-- 
1.6.0.2.763.g72663

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Johannes Sixt]

Johannes Schindelin schrieb:
> Now, git show <tree> shows some more information, and with the -r option,
> it recurses into subdirectories.
> 
> Requested by Scott Chacon.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> 	The only quirk is the command line parsing for -r: we cannot use 
> 	DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched 
> 	on not only by cmd_log_init(), but implicitly by 
> 	diff_setup_done(), because FORMAT_PATCH is selected by git-show.

This comment should for sure go into the commit message.

-- Hannes

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Johannes Schindelin]

Hi,

On Wed, 29 Oct 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > Now, git show <tree> shows some more information, and with the -r option,
> > it recurses into subdirectories.
> > 
> > Requested by Scott Chacon.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > 
> > 	The only quirk is the command line parsing for -r: we cannot use 
> > 	DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched 
> > 	on not only by cmd_log_init(), but implicitly by 
> > 	diff_setup_done(), because FORMAT_PATCH is selected by git-show.
> 
> This comment should for sure go into the commit message.

Fair enough.

Junio, you want me to resend?

Ciao,
Dscho

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Now, git show <tree> shows some more information, and with the -r option,
> it recurses into subdirectories.
>
> Requested by Scott Chacon.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	The only quirk is the command line parsing for -r: we cannot use 
> 	DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched 
> 	on not only by cmd_log_init(), but implicitly by 
> 	diff_setup_done(), because FORMAT_PATCH is selected by git-show.

That's a rather large quirk with an ugly workaround if you ask me.

I also notice that there is:

    int cmd_log_reflog(int argc, const char **argv, const char *prefix)
    {
            struct rev_info rev;
            ...
            /*
             * This means that we override whatever commit format the user gave
             * on the cmd line.  Sad, but cmd_log_init() currently doesn't
             * allow us to set a different default.
             */

I wonder if it would help breaking down cmd_log_init() a bit like this.

 builtin-log.c |   27 +++++++++++++++++++++------
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git c/builtin-log.c w/builtin-log.c
index 2efe593..0fcc28a 100644
--- c/builtin-log.c
+++ w/builtin-log.c
@@ -50,18 +50,23 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	return 0;
 }
 
-static void cmd_log_init(int argc, const char **argv, const char *prefix,
-		      struct rev_info *rev)
+static void cmd_log_init_0(int argc, const char **argv, const char *prefix,
+			   struct rev_info *rev,
+			   int default_abbrev,
+			   int default_commit_format,
+			   int default_verbose_header,
+			   int default_recursive)
 {
 	int i;
 	int decorate = 0;
 
-	rev->abbrev = DEFAULT_ABBREV;
-	rev->commit_format = CMIT_FMT_DEFAULT;
+	rev->abbrev = default_abbrev;
+	rev->commit_format = default_commit_format;
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
-	rev->verbose_header = 1;
-	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
+	rev->verbose_header = default_verbose_header;
+	if (default_recursive)
+		DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 
@@ -88,6 +93,16 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	}
 }
 
+static void cmd_log_init(int argc, const char **argv, const char *prefix,
+			 struct rev_info *rev)
+{
+	cmd_log_init_0(argc, argv, prefix, rev,
+		       DEFAULT_ABBREV,
+		       CMIT_FMT_DEFAULT,
+		       1,
+		       1);
+}
+
 /*
  * This gives a rough estimate for how many commits we
  * will print out in the list.

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Johannes Schindelin]

Hi,

On Thu, 30 Oct 2008, Junio C Hamano wrote:

> I wonder if it would help breaking down cmd_log_init() a bit like this.

Sorry, I am quite busy (this is the first time I am able to check my mail 
since the GitTogether), so I cannot look at that in detail.

However, I strongly expect your suggestion not to help: for showing 
commits, we _want_ recursive to be the default.  And switching that on 
devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if the 
user said '-r' _explicitely_.

Ciao,
Dscho

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Thu, 30 Oct 2008, Junio C Hamano wrote:
>
>> I wonder if it would help breaking down cmd_log_init() a bit like this.
>
> Sorry, I am quite busy (this is the first time I am able to check my mail 
> since the GitTogether), so I cannot look at that in detail.
>
> However, I strongly expect your suggestion not to help: for showing 
> commits, we _want_ recursive to be the default.  And switching that on 
> devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if the 
> user said '-r' _explicitely_.

You can turn on recursive unconditionally for the normal "show committish"
case, and check for explicit "-r" for "show treeish" that was bolted-on
much later, can't you?

Re: [PATCH] git show <tree>: show mode and hash, and handle -r

[Johannes Schindelin]

Hi,

On Fri, 31 Oct 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Thu, 30 Oct 2008, Junio C Hamano wrote:
> >
> >> I wonder if it would help breaking down cmd_log_init() a bit like this.
> >
> > Sorry, I am quite busy (this is the first time I am able to check my 
> > mail since the GitTogether), so I cannot look at that in detail.
> >
> > However, I strongly expect your suggestion not to help: for showing 
> > commits, we _want_ recursive to be the default.  And switching that on 
> > devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if 
> > the user said '-r' _explicitely_.
> 
> You can turn on recursive unconditionally for the normal "show 
> committish" case, and check for explicit "-r" for "show treeish" that 
> was bolted-on much later, can't you?

No, I can't, because cmd_show() uses setup_revisions() (actually, this 
is called by cmd_log_init()) not only to parse the command line arguments, 
but also the objects to show.

The only way I could imagine this working is to turn _off_ FORMAT_PATCH, 
do the parsing, then check if RECURSIVE was set, then turn _on_ 
FORMAT_PATCH and call diff_setup_done().

But that feels just as awful.

Ciao,
Dscho