{"thread":{"id":"16075","subject":"[PATCH] git show : show mode and hash, and handle -r","startedAt":"2008-10-29T01:09:55Z","lastAt":"2008-10-31T21:40:29Z","messageCount":7,"participants":["Johannes Schindelin","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"94160","messageId":"alpine.DEB.1.00.0810290207330.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16075","inReplyTo":null,"subject":"[PATCH] git show : show mode and hash, and handle -r","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-29T01:09:55Z","receivedAt":"2008-10-29T01:09:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nNow, git show shows some more information, and with the -r option,\nit recurses into subdirectories.\n\nRequested by Scott Chacon.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tThe only quirk is the command line parsing for -r: we cannot use \n\tDIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched \n\ton not only by cmd_log_init(), but implicitly by \n\tdiff_setup_done(), because FORMAT_PATCH is selected by git-show.\n\n builtin-log.c | 16 +++++++++++++---\n 1 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 3796dda..b00d353 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -297,8 +297,12 @@ static int show_tree_object(const unsigned char *sha1,\n \t\tconst char *base, int baselen,\n \t\tconst char *pathname, unsigned mode, int stage, void *context)\n {\n-\tprintf(\"%s%s\\n\", pathname, S_ISDIR(mode) ? \"/\" : \"\");\n-\treturn 0;\n+\tint *recursive = context;\n+\tprintf(\"%06o %s %.*s%s%s\\n\", mode,\n+\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV),\n+\t\t\tbaselen, base,\n+\t\t\tpathname, S_ISDIR(mode) ? \"/\" : \"\");\n+\treturn *recursive ? READ_TREE_RECURSIVE : 0;\n }\n \n int cmd_show(int argc, const char **argv, const char *prefix)\n@@ -306,6 +310,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \tstruct rev_info rev;\n \tstruct object_array_entry *objects;\n \tint i, count, ret = 0;\n+\tint tree_recursive = 0;\n \n \tgit_config(git_log_config, NULL);\n \n@@ -319,6 +324,11 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \trev.always_show_header = 1;\n \trev.ignore_merges = 0;\n \trev.no_walk = 1;\n+\tfor (i = 1; i < argc && strcmp(argv[i], \"--\"); i++)\n+\t\tif (!strcmp(argv[i], \"-r\")) {\n+\t\t\ttree_recursive = 1;\n+\t\t\tbreak;\n+\t\t}\n \tcmd_log_init(argc, argv, prefix, &rev);\n \n \tcount = rev.pending.nr;\n@@ -348,7 +358,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tname,\n \t\t\t\t\tdiff_get_color_opt(&rev.diffopt, DIFF_RESET));\n \t\t\tread_tree_recursive((struct tree *)o, \"\", 0, 0, NULL,\n-\t\t\t\t\tshow_tree_object, NULL);\n+\t\t\t\t\tshow_tree_object, &tree_recursive);\n \t\t\tbreak;\n \t\tcase OBJ_COMMIT:\n \t\t\trev.pending.nr = rev.pending.alloc = 0;\n-- \n1.6.0.2.763.g72663\n"},{"id":"94165","messageId":"49080EA4.9080404@viscovery.net","threadId":"16075","inReplyTo":"alpine.DEB.1.00.0810290207330.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-10-29T07:20:04Z","receivedAt":"2008-10-29T07:20:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Now, git show shows some more information, and with the -r option,\n> it recurses into subdirectories.\n> \n> Requested by Scott Chacon.\n> \n> Signed-off-by: Johannes Schindelin \n> ---\n> \n> \tThe only quirk is the command line parsing for -r: we cannot use \n> \tDIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched \n> \ton not only by cmd_log_init(), but implicitly by \n> \tdiff_setup_done(), because FORMAT_PATCH is selected by git-show.\n\nThis comment should for sure go into the commit message.\n\n-- Hannes\n"},{"id":"94181","messageId":"alpine.DEB.1.00.0810291540390.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16075","inReplyTo":"49080EA4.9080404@viscovery.net","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-29T14:41:01Z","receivedAt":"2008-10-29T14:41:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 29 Oct 2008, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > Now, git show shows some more information, and with the -r option,\n> > it recurses into subdirectories.\n> > \n> > Requested by Scott Chacon.\n> > \n> > Signed-off-by: Johannes Schindelin \n> > ---\n> > \n> > \tThe only quirk is the command line parsing for -r: we cannot use \n> > \tDIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched \n> > \ton not only by cmd_log_init(), but implicitly by \n> > \tdiff_setup_done(), because FORMAT_PATCH is selected by git-show.\n> \n> This comment should for sure go into the commit message.\n\nFair enough.\n\nJunio, you want me to resend?\n\nCiao,\nDscho\n"},{"id":"94340","messageId":"7vtzatdbtk.fsf@gitster.siamese.dyndns.org","threadId":"16075","inReplyTo":"alpine.DEB.1.00.0810290207330.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-30T21:15:19Z","receivedAt":"2008-10-30T21:15:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Now, git show shows some more information, and with the -r option,\n> it recurses into subdirectories.\n>\n> Requested by Scott Chacon.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n>\n> \tThe only quirk is the command line parsing for -r: we cannot use \n> \tDIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched \n> \ton not only by cmd_log_init(), but implicitly by \n> \tdiff_setup_done(), because FORMAT_PATCH is selected by git-show.\n\nThat's a rather large quirk with an ugly workaround if you ask me.\n\nI also notice that there is:\n\n int cmd_log_reflog(int argc, const char **argv, const char *prefix)\n {\n struct rev_info rev;\n ...\n /*\n * This means that we override whatever commit format the user gave\n * on the cmd line. Sad, but cmd_log_init() currently doesn't\n * allow us to set a different default.\n */\n\nI wonder if it would help breaking down cmd_log_init() a bit like this.\n\n builtin-log.c | 27 +++++++++++++++++++++------\n 1 files changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git c/builtin-log.c w/builtin-log.c\nindex 2efe593..0fcc28a 100644\n--- c/builtin-log.c\n+++ w/builtin-log.c\n@@ -50,18 +50,23 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in\n \treturn 0;\n }\n \n-static void cmd_log_init(int argc, const char **argv, const char *prefix,\n-\t\t struct rev_info *rev)\n+static void cmd_log_init_0(int argc, const char **argv, const char *prefix,\n+\t\t\t struct rev_info *rev,\n+\t\t\t int default_abbrev,\n+\t\t\t int default_commit_format,\n+\t\t\t int default_verbose_header,\n+\t\t\t int default_recursive)\n {\n \tint i;\n \tint decorate = 0;\n \n-\trev->abbrev = DEFAULT_ABBREV;\n-\trev->commit_format = CMIT_FMT_DEFAULT;\n+\trev->abbrev = default_abbrev;\n+\trev->commit_format = default_commit_format;\n \tif (fmt_pretty)\n \t\tget_commit_format(fmt_pretty, rev);\n-\trev->verbose_header = 1;\n-\tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n+\trev->verbose_header = default_verbose_header;\n+\tif (default_recursive)\n+\t\tDIFF_OPT_SET(&rev->diffopt, RECURSIVE);\n \trev->show_root_diff = default_show_root;\n \trev->subject_prefix = fmt_patch_subject_prefix;\n \n@@ -88,6 +93,16 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t}\n }\n \n+static void cmd_log_init(int argc, const char **argv, const char *prefix,\n+\t\t\t struct rev_info *rev)\n+{\n+\tcmd_log_init_0(argc, argv, prefix, rev,\n+\t\t DEFAULT_ABBREV,\n+\t\t CMIT_FMT_DEFAULT,\n+\t\t 1,\n+\t\t 1);\n+}\n+\n /*\n * This gives a rough estimate for how many commits we\n * will print out in the list.\n"},{"id":"94418","messageId":"alpine.DEB.1.00.0810311753080.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16075","inReplyTo":"7vtzatdbtk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-31T16:55:03Z","receivedAt":"2008-10-31T16:55:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 30 Oct 2008, Junio C Hamano wrote:\n\n> I wonder if it would help breaking down cmd_log_init() a bit like this.\n\nSorry, I am quite busy (this is the first time I am able to check my mail \nsince the GitTogether), so I cannot look at that in detail.\n\nHowever, I strongly expect your suggestion not to help: for showing \ncommits, we _want_ recursive to be the default. And switching that on \ndevoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if the \nuser said '-r' _explicitely_.\n\nCiao,\nDscho\n"},{"id":"94428","messageId":"7vk5bo8y3m.fsf@gitster.siamese.dyndns.org","threadId":"16075","inReplyTo":"alpine.DEB.1.00.0810311753080.22125@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-31T17:37:33Z","receivedAt":"2008-10-31T17:37:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Thu, 30 Oct 2008, Junio C Hamano wrote:\n>\n>> I wonder if it would help breaking down cmd_log_init() a bit like this.\n>\n> Sorry, I am quite busy (this is the first time I am able to check my mail \n> since the GitTogether), so I cannot look at that in detail.\n>\n> However, I strongly expect your suggestion not to help: for showing \n> commits, we _want_ recursive to be the default. And switching that on \n> devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if the \n> user said '-r' _explicitely_.\n\nYou can turn on recursive unconditionally for the normal \"show committish\"\ncase, and check for explicit \"-r\" for \"show treeish\" that was bolted-on\nmuch later, can't you?\n"},{"id":"94462","messageId":"alpine.DEB.1.00.0810312236490.22125@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"16075","inReplyTo":"7vk5bo8y3m.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git show : show mode and hash, and handle -r","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-10-31T21:40:29Z","receivedAt":"2008-10-31T21:40:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 31 Oct 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Thu, 30 Oct 2008, Junio C Hamano wrote:\n> >\n> >> I wonder if it would help breaking down cmd_log_init() a bit like this.\n> >\n> > Sorry, I am quite busy (this is the first time I am able to check my \n> > mail since the GitTogether), so I cannot look at that in detail.\n> >\n> > However, I strongly expect your suggestion not to help: for showing \n> > commits, we _want_ recursive to be the default. And switching that on \n> > devoids us from being able to DIFF_OPT_TST(.., RECURSIVE) to detect if \n> > the user said '-r' _explicitely_.\n> \n> You can turn on recursive unconditionally for the normal \"show \n> committish\" case, and check for explicit \"-r\" for \"show treeish\" that \n> was bolted-on much later, can't you?\n\nNo, I can't, because cmd_show() uses setup_revisions() (actually, this \nis called by cmd_log_init()) not only to parse the command line arguments, \nbut also the objects to show.\n\nThe only way I could imagine this working is to turn _off_ FORMAT_PATCH, \ndo the parsing, then check if RECURSIVE was set, then turn _on_ \nFORMAT_PATCH and call diff_setup_done().\n\nBut that feels just as awful.\n\nCiao,\nDscho\n"}]}