Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Duy Nguyen <pclouds@gmail.com>]

On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
> Changes:
> 
> - remove the deprecated column in command-list.txt. My change break it
>   anyway if anyone uses it.
> - fix up failed tests that I marked in the RFC and kinda forgot about it.
> - fix bashisms in generate-cmdlist.sh
> - fix segfaul in "git help"

Sorry I forgot the interdiff

diff --git a/command-list.txt b/command-list.txt
index 0809a19184..1835f1a928 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -9,7 +9,7 @@ history      grow, mark and tweak your common history
 remote       collaborate (see also: git help workflows)
 
 ### command list (do not change this line)
-# command name                          category [deprecated] [common]
+# command name                          category                [common]
 git-add                                 mainporcelain           worktree
 git-am                                  mainporcelain
 git-annotate                            ancillaryinterrogators
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f17703aa7..7d17ca23f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -835,19 +835,23 @@ __git_complete_strategy ()
 }
 
 __git_commands () {
-	if test -n "${GIT_TESTING_COMMAND_COMPLETION:-}"
+	if test -n "$GIT_TESTING_COMPLETION"
 	then
-		printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
+		case "$1" in
+		porcelain)
+			printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST";;
+		all)
+			printf "%s" "$GIT_TESTING_ALL_COMMAND_LIST";;
+		esac
 	else
 		git --list-cmds=$1
 	fi
 }
 
-__git_list_all_commands ()
+__git_list_commands ()
 {
 	local i IFS=" "$'\n'
-	local category=${1-all}
-	for i in $(__git_commands $category)
+	for i in $(__git_commands $1)
 	do
 		case $i in
 		*--*)             : helper pattern;;
@@ -860,14 +864,14 @@ __git_all_commands=
 __git_compute_all_commands ()
 {
 	test -n "$__git_all_commands" ||
-	__git_all_commands=$(__git_list_all_commands)
+	__git_all_commands=$(__git_list_commands all)
 }
 
 __git_porcelain_commands=
 __git_compute_porcelain_commands ()
 {
 	test -n "$__git_porcelain_commands" ||
-	__git_porcelain_commands=$(__git_list_all_commands porcelain)
+	__git_porcelain_commands=$(__git_list_commands porcelain)
 }
 
 # Lists all set config variables starting with the given section prefix,
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index e35f3e357b..86d59419b3 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -36,7 +36,7 @@ sed -n '
 	' "$1"
 printf '};\n\n'
 
-echo "#define GROUP_NONE 0xff /* no common group */"
+echo "#define GROUP_NONE 0xff"
 n=0
 while read grp
 do
@@ -45,15 +45,6 @@ do
 done <"$grps"
 echo
 
-echo '/*'
-printf 'static const char *cmd_categories[] = {\n'
-category_list "$1" |
-while read category; do
-	printf '\t\"'$category'\",\n'
-done
-printf '\tNULL\n};\n\n'
-echo '*/'
-
 n=0
 category_list "$1" |
 while read category; do
@@ -68,10 +59,11 @@ sort |
 while read cmd category tags
 do
 	if [ "$category" = guide ]; then
-		name=${cmd/git}
+		prefix=git
 	else
-		name=${cmd/git-}
+		prefix=git-
 	fi
+	name=$(echo $cmd | sed "s/^${prefix}//")
 	sed -n '
 		/^NAME/,/'"$cmd"'/H
 		${
diff --git a/help.c b/help.c
index a44f4a113e..88127fdd6f 100644
--- a/help.c
+++ b/help.c
@@ -201,7 +201,8 @@ static void extract_common_cmds(struct cmdname_help **p_common_cmds,
 	for (i = 0; i < ARRAY_SIZE(command_list); i++) {
 		const struct cmdname_help *cmd = command_list + i;
 
-		if (cmd->category != CAT_mainporcelain)
+		if (cmd->category != CAT_mainporcelain ||
+		    cmd->group == GROUP_NONE)
 			continue;
 
 		common_cmds[nr++] = *cmd;
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index fd2a7f27dc..53208ab20e 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -25,6 +25,15 @@ test_expect_success "setup" '
 	EOF
 '
 
+# make sure to exercise these code paths, the output is a bit tricky
+# to verify
+test_expect_success 'basic help commands' '
+	git help >/dev/null &&
+	git help -a >/dev/null &&
+	git help -g >/dev/null &&
+	git help -av >/dev/null
+'
+
 test_expect_success "works for commands and guides by default" '
 	configure_help &&
 	git help status &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4bfd26ddf9..5a23a46fcf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -13,7 +13,7 @@ complete ()
 	return 0
 }
 
-# Be careful when updating this list:
+# Be careful when updating these lists:
 #
 # (1) The build tree may have build artifact from different branch, or
 #     the user's $PATH may have a random executable that may begin
@@ -30,7 +30,9 @@ complete ()
 #     completion for "git <TAB>", and a plumbing is excluded.  "add",
 #     "filter-branch" and "ls-files" are listed for this.
 
-GIT_TESTING_COMMAND_COMPLETION='add checkout check-attr filter-branch ls-files'
+GIT_TESTING_COMPLETION=t
+GIT_TESTING_ALL_COMMAND_LIST='add checkout check-attr filter-branch ls-files'
+GIT_TESTING_PORCELAIN_COMMAND_LIST='add checkout filter-branch'
 
 . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
 
@@ -1208,6 +1210,8 @@ test_expect_success 'basic' '
 	grep -q "^add \$" out &&
 	# script
 	grep -q "^filter-branch \$" out &&
+	# plumbing
+	! grep -q "^ls-files \$" out &&
 
 	run_completion "git f" &&
 	! grep -q -v "^f" out
@@ -1270,7 +1274,7 @@ test_expect_success 'general options' '
 	test_completion "git --no-r" "--no-replace-objects "
 '
 
-test_expect_failure 'general options plus command' '
+test_expect_success 'general options plus command' '
 	test_completion "git --version check" "checkout " &&
 	test_completion "git --paginate check" "checkout " &&
 	test_completion "git --git-dir=foo check" "checkout " &&