{"thread":{"id":"11599","subject":"[PATCH 4/5] git-status: submodule summary support","startedAt":"2008-01-12T07:37:10Z","lastAt":"2008-01-13T06:38:52Z","messageCount":19,"participants":["Ping Yin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"65149","messageId":"1200123435-16722-1-git-send-email-pkufranky@gmail.com","threadId":"11599","inReplyTo":null,"subject":"[PATCH 0/5] submodule summary support","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:10Z","receivedAt":"2008-01-12T07:37:10Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"This patch series introduces summary support for submodule.\n\nThe first three patches teach git-submodule subcommand  'summary', and the last two patches teach git-status/git-commit 'status.submodulesummary' configuration to enable/disable submodule summary.\n"},{"id":"65147","messageId":"f67f45eeb9648bb7e5adaf53544443b79643914e.1200122041.git.pkufranky@gmail.com","threadId":"11599","inReplyTo":"1200123435-16722-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH 1/5] git-submodule: New subcommand 'summary' (1) - code framework","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:11Z","receivedAt":"2008-01-12T07:37:11Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Following patches teach git-submodule a new subcommand 'summary' to show\ncommit summary of user-cared (i.e. checked out) submodules since a given\ncommit (default HEAD) of super project.\n\nFor a submodule in question, a series of commits will be shown as the path\nfrom the src commit to the dst commit, where the src commit is from the given\nsuper project commit, and the dst commit is from the index or working tree\n(switched by --cached).\n\n.Example: a super project with modified/deleted submodules sm1 to sm3.\n--------------------------------------------\n $ git submodule summary\n # Submodules modifiled: sm1 sm2 sm3\n #\n # * sm1 354cd45...3f751e5:\n #   <one line message for C\n #   <one line message for B\n #   >one line message for D\n #   >one line message for E\n #\n # * sm2 5c8bfb5...000000:\n #   <one line message for F\n #\n # * sm3 354cd45...3f751e5:\n #   Warn: sm3 doesn't contain commit 354cd45\n #\n--------------------------------------------\n\nsm1 has commit C as src (given commit or HEAD) and commit E as dst (index\nor working tree) as following picture shows.\n\t--A-->B-->C (in src:354cd45)\n\t  \\\n\t   -->D-->E (in dst:3f751e5)\n\nThe 'Submodules modified' section for sm1 shows how to change sm1 from\nsrc commit C to dst commit E: firstly backward (<) to commit A from\ncommit C via commit B, and then forward (>) to commit E via commit D.\n\nIllustration for output of deleted sm2 is similar.\n\nIf the src/dst commit for a submodule is not found in the work tree (say\nthe submodule respository in the work tree), a warning will be issued.\nsm3 falls into this case.\n\nThis patch just gives the framework. It just finds the submodules to be\nshown as follows.\n\n--------------------------------------------\n $ git submodule summary\n # Submodules modifiled: sm1 sm2 sm3\n #\n--------------------------------------------\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n git-submodule.sh |   57 ++++++++++++++++++++++++++++++++++++++++++++++++-----\n 1 files changed, 51 insertions(+), 6 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ad9fe62..0e81484 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -4,7 +4,7 @@\n #\n # Copyright (c) 2007 Lars Hjemli\n \n-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'\n+USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update|summary <commit>] [--] [<path>...]'\n OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n@@ -255,7 +255,46 @@ set_name_rev () {\n \t) )\n \ttest -z \"$revname\" || revname=\" ($revname)\"\n }\n+#\n+# Show commit summary for submodules in index or working tree relative to the given commit\n+#\n+# If '--cached' is given, show summary between index and given commit,\n+# or between working tree and given commit\n+#\n+# @ = [head counting commits from (default 'HEAD'),] requested paths (default to all)\n+#\n+modules_summary()\n+{\n+\tcache_option=${cached:+--cached}\n+\n+\tif rev=$(git rev-parse --verify \"$1^0\" 2>/dev/null)\n+\tthen\n+\t\thead=$rev\n+\t\tshift\n+\telse\n+\t\thead=HEAD\n+\tfi\n \n+\tcwd=$(pwd)\n+\tcd_to_toplevel\n+\n+\t# get modified modules which have been checked out (i.e. cared by user)\n+\tmodules=$(git diff $cache_option --raw $head -- \"$@\" |\n+\t\tgrep '^:160000\\|:000000 160000' |\n+\t\twhile read mod_src mod_dst sha1_src sha1_dst status name\n+\t\tdo\n+\t\t\ttest $status=D && echo \"$name\" && continue\n+\t\t\tGIT_DIR=\"$name/.git\" git-rev-parse --git-dir >/dev/null 2>&1 &&\n+\t\t\techo \"$name\"\n+\t\tdone\n+\t)\n+\n+\t# TODO: quote module names containing space or tab\n+\ttest -n \"$modules\" &&\n+\techo \"# Submodules modified: \"$modules &&\n+\techo \"#\"\n+\tcd \"$cwd\"\n+}\n #\n # List all submodules, prefixed with:\n #  - submodule not initialized\n@@ -305,6 +344,9 @@ do\n \tupdate)\n \t\tupdate=1\n \t\t;;\n+\tsummary)\n+\t\tsummary=1\n+\t\t;;\n \tstatus)\n \t\tstatus=1\n \t\t;;\n@@ -345,17 +387,20 @@ case \"$add,$branch\" in\n \t;;\n esac\n \n-case \"$add,$init,$update,$status,$cached\" in\n-1,,,,)\n+case \"$add,$init,$update,$summary,$status,$cached\" in\n+1,,,,,)\n \tmodule_add \"$@\"\n \t;;\n-,1,,,)\n+,1,,,,)\n \tmodules_init \"$@\"\n \t;;\n-,,1,,)\n+,,1,,,)\n \tmodules_update \"$@\"\n \t;;\n-,,,*,*)\n+,,,1,,*)\n+\tmodules_summary \"$@\"\n+\t;;\n+,,,,*,*)\n \tmodules_list \"$@\"\n \t;;\n *)\n-- \n1.5.4.rc2.9.gf5146-dirty\n"},{"id":"65150","messageId":"62a73e734832ad67e89be706f1f8b3dbc30cfcf4.1200122041.git.pkufranky@gmail.com","threadId":"11599","inReplyTo":"f67f45eeb9648bb7e5adaf53544443b79643914e.1200122041.git.pkufranky@gmail.com","subject":"[PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:12Z","receivedAt":"2008-01-12T07:37:12Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"This patch does the hard work of submodule summary and finally gives\noutput shown in last patch.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n git-submodule.sh |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 67 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0e81484..cccb539 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -292,7 +292,73 @@ modules_summary()\n \t# TODO: quote module names containing space or tab\n \ttest -n \"$modules\" &&\n \techo \"# Submodules modified: \"$modules &&\n-\techo \"#\"\n+\techo \"#\" &&\n+\tgit diff $cache_option --raw $head -- $modules |\n+\tgrep '^:160000\\|:000000 160000' |\n+\tcut -c2- |\n+\twhile read mod_src mod_dst sha1_src sha1_dst status name\n+\tdo\n+\t\tsha1_dst=$(echo $sha1_dst | cut -c1-7)\n+\t\tsha1_src=$(echo $sha1_src | cut -c1-7)\n+\t\tcheck_dst=t\n+\t\tcheck_src=t\n+\t\tcase $status in\n+\t\tD)\n+\t\t\tcheck_dst=\n+\t\t\t;;\n+\t\tA)\n+\t\t\tcheck_src=\n+\t\t\t;;\n+\t\tesac\n+\n+\t\t(\n+\t\t\terrmsg=\n+\t\t\tunfound_src=\n+\t\t\tunfound_dst=\n+\n+\t\t\ttest -z \"$check_src\" ||\n+\t\t\tGIT_DIR=\"$name/.git\" git-rev-parse $sha1_src >&/dev/null ||\n+\t\t\tunfound_src=t\n+\n+\t\t\ttest -z \"$check_dst\" ||\n+\t\t\tGIT_DIR=\"$name/.git\" git-rev-parse $sha1_dst >&/dev/null ||\n+\t\t\tunfound_dst=t\n+\n+\t\t\tcase \"$unfound_src,$unfound_dst\" in\n+\t\t\tt,)\n+\t\t\t\terrmsg=\"  Warn: $name doesn't contain commit $sha1_src\"\n+\t\t\t\t;;\n+\t\t\t,t)\n+\t\t\t\terrmsg=\"  Warn: $name doesn't contain commit $sha1_dst\"\n+\t\t\t\t;;\n+\t\t\tt,t)\n+\t\t\t\terrmsg=\"  Warn: $name doesn't contain commits $sha1_src and $sha1_dst\"\n+\t\t\t\t;;\n+\t\t\t*)\n+\t\t\t\tleft=\n+\t\t\t\tright=\n+\t\t\t\ttest -n \"$check_src\" &&\n+\t\t\t\tleft=$(GIT_DIR=\"$name/.git\" git log --pretty=format:\"  <%s\" \\\n+\t\t\t\t${check_dst:+$sha1_dst..}$sha1_src 2>/dev/null)\n+\n+\t\t\t\ttest -n \"$check_dst\" &&\n+\t\t\t\tright=$(GIT_DIR=\"$name/.git\" git log --reverse --pretty=format:\"  >%s\" \\\n+\t\t\t\t${check_src:+$sha1_src..}$sha1_dst 2>/dev/null)\n+\t\t\t\t;;\n+\t\t\tesac\n+\n+\t\t\techo \"* $name $sha1_src...$sha1_dst:\"\n+\t\t\tif test -n \"$errmsg\"\n+\t\t\tthen\n+\t\t\t\techo \"$errmsg\"\n+\t\t\telse\n+\t\t\t\ttest -n \"$left\" && echo \"$left\"\n+\t\t\t\ttest -n \"$right\" && echo \"$right\"\n+\t\t\tfi\n+\t\t\techo\n+\t\t) | sed 's/^/# /'\n+\tdone\n+\n \tcd \"$cwd\"\n }\n #\n-- \n1.5.4.rc2.9.gf5146-dirty\n"},{"id":"65148","messageId":"bce0ee9fde688c27fc788fab91a571184918da67.1200122041.git.pkufranky@gmail.com","threadId":"11599","inReplyTo":"62a73e734832ad67e89be706f1f8b3dbc30cfcf4.1200122041.git.pkufranky@gmail.com","subject":"[PATCH 3/5] git-submodule: New subcommand 'summary' (3) - limit summary size","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:13Z","receivedAt":"2008-01-12T07:37:13Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"This patches teaches git-submodule an option '--summary-limit|-n <number>'\nto limit number of commits for the summary. Number 0 will disable summary\nand minus number will not limit the summary size.\n\nFor beauty and clarification, the last commit for each section (backward\nand forward) will always be shown disregarding the given limit. So actual\nsummary size may be greater than the given limit.\n\nIn the same super project of these patch series, 'git submodule -n 2\nsummary sm1' and 'git submodule -n 3 summary sm1' will show the same.\n\n---------------------------------------\n $ git submodule -n 2 summary sm1\n # Submodules modifiled: sm1\n #\n # * sm1 354cd45...3f751e5:\n #   <one line message for C\n #   <one line message for B\n #   >... (1 more)\n #   >one line message for E\n #\n---------------------------------------\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n git-submodule.sh |   40 +++++++++++++++++++++++++++++++++++++---\n 1 files changed, 37 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex cccb539..58d0243 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -4,7 +4,8 @@\n #\n # Copyright (c) 2007 Lars Hjemli\n \n-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update|summary <commit>] [--] [<path>...]'\n+USAGE=\"[--quiet] [--cached] [-n|--summary-limit <number>] \\\n+[add <repo> [-b branch]|status|init|update|summary <commit>] [--] [<path>...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n@@ -16,6 +17,7 @@ update=\n status=\n quiet=\n cached=\n+summary_limit=\n \n #\n # print stuff on stdout unless -q was specified\n@@ -265,6 +267,10 @@ set_name_rev () {\n #\n modules_summary()\n {\n+\tsummary_limit=${summary_limit:-1000000}\n+\tsummary_limit=$((summary_limit<0?1000000:summary_limit))\n+\ttest $summary_limit = 0 && return\n+\n \tcache_option=${cached:+--cached}\n \n \tif rev=$(git rev-parse --verify \"$1^0\" 2>/dev/null)\n@@ -352,8 +358,29 @@ modules_summary()\n \t\t\tthen\n \t\t\t\techo \"$errmsg\"\n \t\t\telse\n-\t\t\t\ttest -n \"$left\" && echo \"$left\"\n-\t\t\t\ttest -n \"$right\" && echo \"$right\"\n+\t\t\t\tlc0=0\n+\t\t\t\trc0=0\n+\t\t\t\ttest -n \"$left\" && lc0=$(echo \"$left\" | wc -l)\n+\t\t\t\ttest -n \"$right\" && rc0=$(echo \"$right\" | wc -l)\n+\t\t\t\tlc=$((lc0<summary_limit?lc0:summary_limit))\n+\t\t\t\trc=$((summary_limit>lc?summary_limit-lc:1))\n+\t\t\t\trc=$((rc<rc0?rc:rc0))\n+\n+\t\t\t\tif test -n \"$left\"\n+\t\t\t\tthen\n+\t\t\t\t\tskip=$((lc0-lc))\n+\t\t\t\t\techo \"$left\" | head -$((lc-1))\n+\t\t\t\t\ttest $skip != 0 && echo \"  <... ($skip more)\"\n+\t\t\t\t\techo \"$left\" | tail -1\n+\t\t\t\tfi\n+\n+\t\t\t\tif test -n \"$right\"\n+\t\t\t\tthen\n+\t\t\t\t\tskip=$((rc0-rc))\n+\t\t\t\t\techo \"$right\" | head -$((rc-1))\n+\t\t\t\t\ttest $skip != 0 && echo \"  >... ($skip more)\"\n+\t\t\t\t\techo \"$right\" | tail -1\n+\t\t\t\tfi\n \t\t\tfi\n \t\t\techo\n \t\t) | sed 's/^/# /'\n@@ -430,6 +457,13 @@ do\n \t--cached)\n \t\tcached=1\n \t\t;;\n+\t-n|--summary-limit)\n+\t\tif [ -z \"$2\" ]; then\n+\t\t\tusage\n+\t\tfi\n+\t\tsummary_limit=\"$2\"\n+\t\tshift\n+\t\t;;\n \t--)\n \t\tbreak\n \t\t;;\n-- \n1.5.4.rc2.9.gf5146-dirty\n"},{"id":"65146","messageId":"b220c31f973576dfc1a90ee588bbfd01cd8f6409.1200122041.git.pkufranky@gmail.com","threadId":"11599","inReplyTo":"bce0ee9fde688c27fc788fab91a571184918da67.1200122041.git.pkufranky@gmail.com","subject":"[PATCH 4/5] git-status: submodule summary support","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:14Z","receivedAt":"2008-01-12T07:37:14Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"By introducing this commit, 'git commit/status' will additionally show\n'Submodules modified' section given by 'git --cached submodule summary' just\nbefore 'Untracked files' section.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n wt-status.c |   28 ++++++++++++++++++++++++++++\n 1 files changed, 28 insertions(+), 0 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex c0c2472..df5be80 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -7,6 +7,7 @@\n #include \"diff.h\"\n #include \"revision.h\"\n #include \"diffcore.h\"\n+#include \"run-command.h\"\n \n int wt_status_relative_paths = 1;\n int wt_status_use_color = 0;\n@@ -271,6 +272,30 @@ static void wt_status_print_changed(struct wt_status *s)\n \twt_read_cache(s);\n \trun_diff_files(&rev, 0);\n }\n+static void wt_status_print_submodule_summary(struct wt_status *s)\n+{\n+\tstruct child_process sm_summary;\n+\tconst char *argv[] = {\n+\t\t\"submodule\",\n+\t\t\"summary\",\n+\t\t\"--cached\",\n+\t\ts->amend ? \"HEAD^\" : \"HEAD\",\n+\t\tNULL\n+\t};\n+\tchar index[PATH_MAX];\n+\tconst char *env[2] = { index, NULL };\n+\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", s->index_file);\n+\n+\tmemset(&sm_summary, 0, sizeof(sm_summary));\n+\tsm_summary.argv = argv;\n+\tsm_summary.env = env;\n+\tsm_summary.git_cmd = 1;\n+\tsm_summary.no_stdin = 1;\n+\tfflush(s->fp);\n+\tsm_summary.out = dup(fileno(s->fp));    /* run_command closes it */\n+\tsm_summary.no_stderr = 1;\n+\trun_command(&sm_summary);\n+}\n \n static void wt_status_print_untracked(struct wt_status *s)\n {\n@@ -374,6 +399,9 @@ void wt_status_print(struct wt_status *s)\n \t}\n \n \twt_status_print_changed(s);\n+\t// we must flush s->fp here since the following call will write to s->fp in a child process\n+\tfflush(s->fp);\n+\twt_status_print_submodule_summary(s);\n \twt_status_print_untracked(s);\n \n \tif (s->verbose && !s->is_initial)\n-- \n1.5.4.rc2.9.gf5146-dirty\n"},{"id":"65151","messageId":"576f7d00c7ecb2cf92fe22592f04b8b1ea6512f7.1200122041.git.pkufranky@gmail.com","threadId":"11599","inReplyTo":"b220c31f973576dfc1a90ee588bbfd01cd8f6409.1200122041.git.pkufranky@gmail.com","subject":"[PATCH 5/5] git-status: configurable submodule summary size","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T07:37:15Z","receivedAt":"2008-01-12T07:37:15Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Add config variable status.submodulesummary which is passed as\nthe arg for '--summary-limit' of 'git submodule' to limit the\nsubmodule summary size.\n\nThe summary function is disabled by default by this config\n(say status.submodulesummary is 0 by default).\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n wt-status.c |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex df5be80..25ce58b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -11,6 +11,7 @@\n \n int wt_status_relative_paths = 1;\n int wt_status_use_color = 0;\n+int wt_status_submodule_summary = 0;\n static char wt_status_colors[][COLOR_MAXLEN] = {\n \t\"\",         /* WT_STATUS_HEADER: normal */\n \t\"\\033[32m\", /* WT_STATUS_UPDATED: green */\n@@ -275,10 +276,14 @@ static void wt_status_print_changed(struct wt_status *s)\n static void wt_status_print_submodule_summary(struct wt_status *s)\n {\n \tstruct child_process sm_summary;\n+\tchar summary_limit[64];\n+\tsprintf(summary_limit, \"%d\", wt_status_submodule_summary);\n \tconst char *argv[] = {\n \t\t\"submodule\",\n \t\t\"summary\",\n \t\t\"--cached\",\n+\t\t\"--summary-limit\",\n+\t\tsummary_limit,\n \t\ts->amend ? \"HEAD^\" : \"HEAD\",\n \t\tNULL\n \t};\n@@ -424,6 +429,10 @@ void wt_status_print(struct wt_status *s)\n \n int git_status_config(const char *k, const char *v)\n {\n+\tif (!strcmp(k, \"status.submodulesummary\")) {\n+\t\twt_status_submodule_summary = atoi(v);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(k, \"status.color\") || !strcmp(k, \"color.status\")) {\n \t\twt_status_use_color = git_config_colorbool(k, v, -1);\n \t\treturn 0;\n-- \n1.5.4.rc2.9.gf5146-dirty\n"},{"id":"65155","messageId":"7vmyrbv4ms.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"f67f45eeb9648bb7e5adaf53544443b79643914e.1200122041.git.pkufranky@gmail.com","subject":"Re: [PATCH 1/5] git-submodule: New subcommand 'summary' (1) - code framework","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T08:18:19Z","receivedAt":"2008-01-12T08:18:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> +\t# get modified modules which have been checked out (i.e. cared by user)\n> +\tmodules=$(git diff $cache_option --raw $head -- \"$@\" |\n> +\t\tgrep '^:160000\\|:000000 160000' |\n> +\t\twhile read mod_src mod_dst sha1_src sha1_dst status name\n> +\t\tdo\n\nYou are listing paths that were already submodule in HEAD, or\nnewly added submodule.  What about a path that used to be a blob\nbut is being made into submodule with the next commit (i.e. RHS\nis 160000 but LHS is not 000000)?\n\n> +\t# TODO: quote module names containing space or tab\n\nYes, you would need to worry about *un*quoting them.\n\n> +\ttest -n \"$modules\" &&\n> +\techo \"# Submodules modified: \"$modules &&\n> +\techo \"#\"\n> +\tcd \"$cwd\"\n\nHmmmmmm.... \n\n> -case \"$add,$init,$update,$status,$cached\" in\n> -1,,,,)\n> +case \"$add,$init,$update,$summary,$status,$cached\" in\n> +1,,,,,)\n>  \tmodule_add \"$@\"\n>  \t;;\n\nThis is simply unsustainable.\n\nPlease see the other thread with Imran M Yousuf regarding the\ncommand dispatcher.  I think that should be the first thing to\nfix before doing any change.\n"},{"id":"65156","messageId":"7vejcnv3zk.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"62a73e734832ad67e89be706f1f8b3dbc30cfcf4.1200122041.git.pkufranky@gmail.com","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T08:32:15Z","receivedAt":"2008-01-12T08:32:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> +\tgit diff $cache_option --raw $head -- $modules |\n> +\tgrep '^:160000\\|:000000 160000' |\n> +\tcut -c2- |\n> +\twhile read mod_src mod_dst sha1_src sha1_dst status name\n> +\tdo\n> +\t\tsha1_dst=$(echo $sha1_dst | cut -c1-7)\n> +\t\tsha1_src=$(echo $sha1_src | cut -c1-7)\n\nIf you are willing to lose precision forever like this, I think\nyou can run \"git diff\" with --abbrev and lose this cut.\n\n> +\t\tcheck_dst=t\n> +\t\tcheck_src=t\n> +\t\tcase $status in\n> +\t\tD)\n> +\t\t\tcheck_dst=\n> +\t\t\t;;\n> +\t\tA)\n> +\t\t\tcheck_src=\n> +\t\t\t;;\n\nI'd loosen the above grep (see my comments to your 1/5) and also\nadd this:\n\n\t\t*)\n\t\t\tcontinue ;# punt\n\t\t\t;;\n\nso that the rest of the code won't break when seeing a path that\nwas submodule in the HEAD but is a blob in the index.\n\n> +\t\tesac\n> +\n> +\t\t(\n> +\t\t\terrmsg=\n> +\t\t\tunfound_src=\n> +\t\t\tunfound_dst=\n> +\n> +\t\t\ttest -z \"$check_src\" ||\n> +\t\t\tGIT_DIR=\"$name/.git\" git-rev-parse $sha1_src >&/dev/null ||\n\nAnd the precision of $sha1_src matter here.  Be it done with\n\"diff --abbrev\" at the toplevel or your \"cut\", that may not be\nunique enough in the submodule.\n\nI think you would want to read full 40-char sha1_src and\nsha1_dst with \"while read\", and keep that full 40-char in these\nvariables, and use them when calling rev-parse here.\n\nIf you are checking if that the object exists in the submodule,\nuse \"rev-parse --verify\", which was designed for exactly that\npurpose.  If you also want to verify if the object is a commit,\nwhich may be a good idea anyway, \"rev-parse --verify $sha1_src^0\".\n\nTo be portable, use traditional \">/dev/null 2>&1\", not \">&/dev/null\".\n\n> +\t\t\tcase \"$unfound_src,$unfound_dst\" in\n> +\t\t\tt,)\n> +\t\t\t\terrmsg=\"  Warn: $name doesn't contain commit $sha1_src\"\n> +\t\t\t\t;;\n> +\t\t\t,t)\n> +\t\t\t\terrmsg=\"  Warn: $name doesn't contain commit $sha1_dst\"\n> +\t\t\t\t;;\n> +\t\t\tt,t)\n> +\t\t\t\terrmsg=\"  Warn: $name doesn't contain commits $sha1_src and $sha1_dst\"\n> +\t\t\t\t;;\n\nWhen reporting errors, you would want to give full 40-chars...\n\n> +\t\t\t*)\n> +\t\t\t\tleft=\n> +\t\t\t\tright=\n> +\t\t\t\ttest -n \"$check_src\" &&\n> +\t\t\t\tleft=$(GIT_DIR=\"$name/.git\" git log --pretty=format:\"  <%s\" \\\n> +\t\t\t\t${check_dst:+$sha1_dst..}$sha1_src 2>/dev/null)\n> +\n> +\t\t\t\ttest -n \"$check_dst\" &&\n> +\t\t\t\tright=$(GIT_DIR=\"$name/.git\" git log --reverse --pretty=format:\"  >%s\" \\\n> +\t\t\t\t${check_src:+$sha1_src..}$sha1_dst 2>/dev/null)\n> +\t\t\t\t;;\n> +\t\t\tesac\n> +\n> +\t\t\techo \"* $name $sha1_src...$sha1_dst:\"\n\nWhile reporting like this, you would want the shortened form,\nperhaps produced your \"cut -c1-7\".\n\n> +\t\t\tif test -n \"$errmsg\"\n> +\t\t\tthen\n> +\t\t\t\techo \"$errmsg\"\n> +\t\t\telse\n> +\t\t\t\ttest -n \"$left\" && echo \"$left\"\n> +\t\t\t\ttest -n \"$right\" && echo \"$right\"\n> +\t\t\tfi\n> +\t\t\techo\n> +\t\t) | sed 's/^/# /'\n> +\tdone\n\nI'd prefer to always have \"-e\" before the sed expression.\n\nAny reason why you want separate invocation of sed inside the\nwhile loop?  IOW, why isn't it like this?\n\n\tgit diff --raw |\n        while read ...\n        do\n        \t...\n\tdone | sed -e 's/^/# /'\n\n> +\n>  \tcd \"$cwd\"\n\nHmmm.\n"},{"id":"65157","messageId":"7vabnbv3sb.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"bce0ee9fde688c27fc788fab91a571184918da67.1200122041.git.pkufranky@gmail.com","subject":"Re: [PATCH 3/5] git-submodule: New subcommand 'summary' (3) - limit summary size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T08:36:36Z","receivedAt":"2008-01-12T08:36:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> @@ -265,6 +267,10 @@ set_name_rev () {\n>  #\n>  modules_summary()\n>  {\n> +\tsummary_limit=${summary_limit:-1000000}\n\nWhy a million?\n\n> +\tsummary_limit=$((summary_limit<0?1000000:summary_limit))\n\nThis is doubly bashism.  Variables must be referenced with $,\nand $(( conditional ? iftrue : iffalse )) is not POSIX.\n"},{"id":"65160","messageId":"46dff0320801120109l5d69fcbancd68bbcec7f2455a@mail.gmail.com","threadId":"11599","inReplyTo":"7vmyrbv4ms.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/5] git-submodule: New subcommand 'summary' (1) - code framework","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T09:09:49Z","receivedAt":"2008-01-12T09:09:49Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Jan 12, 2008 4:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n>\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n> > +     # get modified modules which have been checked out (i.e. cared by user)\n> > +     modules=$(git diff $cache_option --raw $head -- \"$@\" |\n> > +             grep '^:160000\\|:000000 160000' |\n> > +             while read mod_src mod_dst sha1_src sha1_dst status name\n> > +             do\n>\n> You are listing paths that were already submodule in HEAD, or\n> newly added submodule.  What about a path that used to be a blob\n> but is being made into submodule with the next commit (i.e. RHS\n> is 160000 but LHS is not 000000)?\n\nHmm, i had ignored such a case.\n>\n>\n>\n> > -case \"$add,$init,$update,$status,$cached\" in\n> > -1,,,,)\n> > +case \"$add,$init,$update,$summary,$status,$cached\" in\n> > +1,,,,,)\n> >       module_add \"$@\"\n> >       ;;\n>\n> This is simply unsustainable.\n>\n> Please see the other thread with Imran M Yousuf regarding the\n> command dispatcher.  I think that should be the first thing to\n> fix before doing any change.\n>\nOk, i will resend my patches after this fix\n\n\n-- \nPing Yin\n"},{"id":"65162","messageId":"46dff0320801120148r1a760bcdq4b9dcdf31354d9e0@mail.gmail.com","threadId":"11599","inReplyTo":"7vejcnv3zk.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T09:48:59Z","receivedAt":"2008-01-12T09:48:59Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Jan 12, 2008 4:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n>\n> > +             check_dst=t\n> > +             check_src=t\n> > +             case $status in\n> > +             D)\n> > +                     check_dst=\n> > +                     ;;\n> > +             A)\n> > +                     check_src=\n> > +                     ;;\n>\n> I'd loosen the above grep (see my comments to your 1/5) and also\n> add this:\n>\n>                 *)\n>                         continue ;# punt\n>                         ;;\n>\n> so that the rest of the code won't break when seeing a path that\n> was submodule in the HEAD but is a blob in the index.\n\nRight. I thought 'M' should be the default case because I missed the\ncase 'T' (head blob but index submodule, or the reverse) .\n\n>\n> > +             esac\n> > +\n> > +             (\n> > +                     errmsg=\n> > +                     unfound_src=\n> > +                     unfound_dst=\n> > +\n> > +                     test -z \"$check_src\" ||\n> > +                     GIT_DIR=\"$name/.git\" git-rev-parse $sha1_src >&/dev/null ||\n>\n>\n> I think you would want to read full 40-char sha1_src and\n> sha1_dst with \"while read\", and keep that full 40-char in these\n> variables, and use them when calling rev-parse here.\n\nHmm, precision is really a problem. However, \"git diff --raw\" will not\nalways give full 40-char sha1, instead it will give sha1 with enough\nlength. So maybe i can use the sha1 from \"git diff --raw\" ?\n\n>\n> If you are checking if that the object exists in the submodule,\n> use \"rev-parse --verify\", which was designed for exactly that\n> purpose.  If you also want to verify if the object is a commit,\n> which may be a good idea anyway, \"rev-parse --verify $sha1_src^0\".\n\nYes, --verify is better.\n                   ;;\n>\n> When reporting errors, you would want to give full 40-chars...\n>\nAs said before, enough is ok?\n\n> > +                     *)\n> > +                             left=\n> > +                             right=\n> > +                             test -n \"$check_src\" &&\n> > +                             left=$(GIT_DIR=\"$name/.git\" git log --pretty=format:\"  <%s\" \\\n> > +                             ${check_dst:+$sha1_dst..}$sha1_src 2>/dev/null)\n> > +\n> > +                             test -n \"$check_dst\" &&\n> > +                             right=$(GIT_DIR=\"$name/.git\" git log --reverse --pretty=format:\"  >%s\" \\\n> > +                             ${check_src:+$sha1_src..}$sha1_dst 2>/dev/null)\n> > +                             ;;\n> > +                     esac\n> > +\n> > +                     echo \"* $name $sha1_src...$sha1_dst:\"\n>\n> While reporting like this, you would want the shortened form,\n> perhaps produced your \"cut -c1-7\".\n>\n> > +                     if test -n \"$errmsg\"\n> > +                     then\n> > +                             echo \"$errmsg\"\n> > +                     else\n> > +                             test -n \"$left\" && echo \"$left\"\n> > +                             test -n \"$right\" && echo \"$right\"\n> > +                     fi\n> > +                     echo\n> > +             ) | sed 's/^/# /'\n> > +     done\n>\n> I'd prefer to always have \"-e\" before the sed expression.\nIs it not portable without \"-e\"?\n\n>\n> Any reason why you want separate invocation of sed inside the\n> while loop?  IOW, why isn't it like this?\n>\n>         git diff --raw |\n>         while read ...\n>         do\n>                 ...\n>         done | sed -e 's/^/# /'\n>\nJust because i'm stupid. :)\n\n\n\n-- \nPing Yin\n"},{"id":"65163","messageId":"46dff0320801120151s7959edddp1e1f8b506da79e4e@mail.gmail.com","threadId":"11599","inReplyTo":"7vabnbv3sb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/5] git-submodule: New subcommand 'summary' (3) - limit summary size","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T09:51:43Z","receivedAt":"2008-01-12T09:51:43Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Jan 12, 2008 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n> > @@ -265,6 +267,10 @@ set_name_rev () {\n> >  #\n> >  modules_summary()\n> >  {\n> > +     summary_limit=${summary_limit:-1000000}\n>\n> Why a million?\nBecause i think a million is big enough. I'd better define a constant\nfor unlimited number.\n>\n> > +     summary_limit=$((summary_limit<0?1000000:summary_limit))\n>\n> This is doubly bashism.  Variables must be referenced with $,\n> and $(( conditional ? iftrue : iffalse )) is not POSIX.\n>\nOk, i'll fix this.\n\n\n-- \nPing Yin\n"},{"id":"65167","messageId":"46dff0320801120312i7b22f13vb9fe2394b1f687a9@mail.gmail.com","threadId":"11599","inReplyTo":"62a73e734832ad67e89be706f1f8b3dbc30cfcf4.1200122041.git.pkufranky@gmail.com","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T11:12:00Z","receivedAt":"2008-01-12T11:12:00Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"> +                       echo \"* $name $sha1_src...$sha1_dst:\"\n\nIf it's a type change (head submodule but index blob, or the the\nreverse), $sha1_dst or $sha1_src will be the sha1 of the blob. It's\ninapprociate to be shown as if it's a commit in the submodule. May\n00000000 should be shown instead of the blob sha1? However, any good\nway to tell user the type change info? Is it ok to add an additional\nline such as \"\"Add file sm1\"  in the summary and also change the\nsummary description as follows?\n\n-------------------------------------------------------------------------------------\n $ git submodule summary sm1\n # Submodules modifiled: sm1\n #\n # * sm1 354cd45...0000000(3f751e5):\n #   <one line message for C\n #   <one line message for B\n #   >Add file sm1\n-------------------------------------------------------------------------------------\n\n\n\n> 1.5.4.rc2.9.gf5146-dirty\n>\n>\n\n\n\n-- \nPing Yin\n"},{"id":"65170","messageId":"46dff0320801120424v1b780a97x8a4ecfcfe8e52f7@mail.gmail.com","threadId":"11599","inReplyTo":"46dff0320801120148r1a760bcdq4b9dcdf31354d9e0@mail.gmail.com","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-12T12:24:09Z","receivedAt":"2008-01-12T12:24:09Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"> >\n> > I think you would want to read full 40-char sha1_src and\n> > sha1_dst with \"while read\", and keep that full 40-char in these\n> > variables, and use them when calling rev-parse here.\n>\n> Hmm, precision is really a problem. However, \"git diff --raw\" will not\n> always give full 40-char sha1, instead it will give sha1 with enough\n> length. So maybe i can use the sha1 from \"git diff --raw\" ?\n>\nOh, I'm wrong. It seems 'git diff --raw' will always give full 40-char\nsha1 for submodule entry and abbreviated sha1 for blob entry.\n\n-- \nPing Yin\n"},{"id":"65209","messageId":"7v4pdiua4k.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"46dff0320801120151s7959edddp1e1f8b506da79e4e@mail.gmail.com","subject":"Re: [PATCH 3/5] git-submodule: New subcommand 'summary' (3) - limit summary size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T19:17:15Z","receivedAt":"2008-01-12T19:17:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n> On Jan 12, 2008 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Ping Yin <pkufranky@gmail.com> writes:\n>>\n>> > @@ -265,6 +267,10 @@ set_name_rev () {\n>> >  #\n>> >  modules_summary()\n>> >  {\n>> > +     summary_limit=${summary_limit:-1000000}\n>>\n>> Why a million?\n> Because i think a million is big enough. I'd better define a constant\n> for unlimited number.\n\nI think that is a wrong approach to begin with.  You are\nassuming that you will always limit and by using improbably\nlarge limit to pretend it is unlimited.  Why not making the\nsummary list generator truely capable of produce an unlimited\nlist?\n\nI also think using 100 or so as a sane default, allowing the\nuser to override to say \"I do not want any limitation\", is a\nmuch better default.\n"},{"id":"65210","messageId":"7vzlvasvcy.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"46dff0320801120424v1b780a97x8a4ecfcfe8e52f7@mail.gmail.com","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T19:21:33Z","receivedAt":"2008-01-12T19:21:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n>> >\n>> > I think you would want to read full 40-char sha1_src and\n>> > sha1_dst with \"while read\", and keep that full 40-char in these\n>> > variables, and use them when calling rev-parse here.\n>>\n>> Hmm, precision is really a problem. However, \"git diff --raw\" will not\n>> always give full 40-char sha1, instead it will give sha1 with enough\n>> length. So maybe i can use the sha1 from \"git diff --raw\" ?\n>>\n> Oh, I'm wrong. It seems 'git diff --raw' will always give full 40-char\n> sha1 for submodule entry and abbreviated sha1 for blob entry.\n\nIt is not recommended to use \"git diff\" in scripts when you can\nuse one of the \"git diff-*\" plumbing.  In this case I think you\nwould want \"git-diff-index\".  Also see --abbrev option.\n\nYou can never determine how many hexdigits are \"enough\" from the\ncontaining project, as it does not have to have access to the\nsubmodule object store.  That's the reason I suggested to read\nfull object name from diff-index and use it for error reporting\nand object retrieval, and shorten it in the UI for normal status\nnoise.\n"},{"id":"65211","messageId":"7vve5ysv72.fsf@gitster.siamese.dyndns.org","threadId":"11599","inReplyTo":"46dff0320801120312i7b22f13vb9fe2394b1f687a9@mail.gmail.com","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T19:25:05Z","receivedAt":"2008-01-12T19:25:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n>> +                       echo \"* $name $sha1_src...$sha1_dst:\"\n>\n> If it's a type change (head submodule but index blob, or the the\n> reverse), $sha1_dst or $sha1_src will be the sha1 of the blob. It's\n> inapprociate to be shown as if it's a commit in the submodule. May\n> 00000000 should be shown instead of the blob sha1?\n\nI do not think that adds much value.  When A or B is a\nnon-commit, you know that A...B notation does not apply, and\nbecause it is probably a rare situation you would want to make\nit even more clearer to the reader by using a different\nnotation.  Like\n\n    echo \"* $name have changed from submodule $sha1_src to blob $sha1_dst!!\".\n\nperhaps in red bold letter in larger font ;-)\n"},{"id":"65229","messageId":"46dff0320801122228r4261786eo2f23258e79f2e346@mail.gmail.com","threadId":"11599","inReplyTo":"7vve5ysv72.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] git-submodule: New subcommand 'summary' (2) - hard work","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-13T06:28:21Z","receivedAt":"2008-01-13T06:28:21Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Jan 13, 2008 3:25 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>\n> >> +                       echo \"* $name $sha1_src...$sha1_dst:\"\n> >\n> > If it's a type change (head submodule but index blob, or the the\n> > reverse), $sha1_dst or $sha1_src will be the sha1 of the blob. It's\n> > inapprociate to be shown as if it's a commit in the submodule. May\n> > 00000000 should be shown instead of the blob sha1?\n>\n> I do not think that adds much value.  When A or B is a\n> non-commit, you know that A...B notation does not apply, and\n> because it is probably a rare situation you would want to make\n> it even more clearer to the reader by using a different\n> notation.  Like\n>\n>     echo \"* $name have changed from submodule $sha1_src to blob $sha1_dst!!\".\n>\n> perhaps in red bold letter in larger font ;-)\n>\nOK, i will think over.\n\n\n\n-- \nPing Yin\n"},{"id":"65230","messageId":"46dff0320801122238k3c2d93f3x4faf2e08f48c43fe@mail.gmail.com","threadId":"11599","inReplyTo":"7v4pdiua4k.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/5] git-submodule: New subcommand 'summary' (3) - limit summary size","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-01-13T06:38:52Z","receivedAt":"2008-01-13T06:38:52Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Jan 13, 2008 3:17 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>\n> > On Jan 12, 2008 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> >> Ping Yin <pkufranky@gmail.com> writes:\n> >>\n> >> > @@ -265,6 +267,10 @@ set_name_rev () {\n> >> >  #\n> >> >  modules_summary()\n> >> >  {\n> >> > +     summary_limit=${summary_limit:-1000000}\n> >>\n> >> Why a million?\n> > Because i think a million is big enough. I'd better define a constant\n> > for unlimited number.\n>\n> I think that is a wrong approach to begin with.  You are\n> assuming that you will always limit and by using improbably\n> large limit to pretend it is unlimited.  Why not making the\n> summary list generator truely capable of produce an unlimited\n> list?\n>\nI used a pseudo unlimited number just to make code shorter.\nAfter considering it again, i find that the code is still brief  by using\na real unlimited number. So i'll correct it.\n\n> I also think using 100 or so as a sane default, allowing the\n> user to override to say \"I do not want any limitation\", is a\n> much better default.\n>\nReasonable\n\n\n\n-- \nPing Yin\n"}]}