t0040,t1502: Demonstrate parse_options bugs

When the option spec contains no switches or only hidden switches,
parse_options will emit an extra blank line at the end of help output so
that the help text will end in two blank lines instead of one.

When parse_options produces internal help output after an error has
occurred it will emit blank lines within the usage string to stdout
instead of stderr.

Update t/helper/test-parse-options.c to have a description body in the
usage string to exercise this second bug and mark tests as failing in
t0040.

Add tests to t1502 to demonstrate both of these problems.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

3 files changed, 107 insertions, 3 deletions

diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 75fe883aac..630c76d127 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -99,6 +99,8 @@ int cmd_main(int argc, const char **argv)
 	const char *prefix = "prefix/";
 	const char *usage[] = {
 		"test-parse-options <options>",
+		"",
+		"A helper function for the parse-options API.",
 		NULL
 	};
 	struct string_list expect = STRING_LIST_INIT_NODUP;
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 74d2cd76fe..a36434bd10 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -10,6 +10,8 @@ test_description='our own option parser'
 cat >expect <<\EOF
 usage: test-parse-options <options>
 
+    A helper function for the parse-options API.
+
     --yes                 get a boolean
     -D, --no-doubt        begins with 'no-'
     -B, --no-fear         be brave
@@ -90,8 +92,8 @@ test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
 test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
 test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
 
-test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown_i18n --fear'
-test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown_i18n --no-no-fear'
+test_expect_failure 'OPT_BOOL() no negation #1' 'check_unknown_i18n --fear'
+test_expect_failure 'OPT_BOOL() no negation #2' 'check_unknown_i18n --no-no-fear'
 
 test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
 
@@ -286,7 +288,7 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '
 
 >expect
 
-test_expect_success 'OPT_CALLBACK() and callback errors work' '
+test_expect_failure 'OPT_CALLBACK() and callback errors work' '
 	test_must_fail test-parse-options --no-length >output 2>output.err &&
 	test_i18ncmp expect output &&
 	test_i18ncmp expect.err output.err
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 6e1b45fd12..1bfa80f8ac 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -38,6 +38,25 @@ test_expect_success 'setup optionspec' '
 EOF
 '
 
+test_expect_success 'setup optionspec-no-switches' '
+	sed -e "s/^|//" >optionspec_no_switches <<\EOF
+|some-command [options] <args>...
+|
+|some-command does foo and bar!
+|--
+EOF
+'
+
+test_expect_success 'setup optionspec-only-hidden-switches' '
+	sed -e "s/^|//" >optionspec_only_hidden_switches <<\EOF
+|some-command [options] <args>...
+|
+|some-command does foo and bar!
+|--
+|hidden1* A hidden switch
+EOF
+'
+
 test_expect_success 'test --parseopt help output' '
 	sed -e "s/^|//" >expect <<\END_EXPECT &&
 |cat <<\EOF
@@ -79,6 +98,87 @@ END_EXPECT
 	test_i18ncmp expect output
 '
 
+test_expect_failure 'test --parseopt help output no switches' '
+	sed -e "s/^|//" >expect <<\END_EXPECT &&
+|cat <<\EOF
+|usage: some-command [options] <args>...
+|
+|    some-command does foo and bar!
+|
+|EOF
+END_EXPECT
+	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_no_switches &&
+	test_i18ncmp expect output
+'
+
+test_expect_failure 'test --parseopt help output hidden switches' '
+	sed -e "s/^|//" >expect <<\END_EXPECT &&
+|cat <<\EOF
+|usage: some-command [options] <args>...
+|
+|    some-command does foo and bar!
+|
+|EOF
+END_EXPECT
+	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_only_hidden_switches &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'test --parseopt help-all output hidden switches' '
+	sed -e "s/^|//" >expect <<\END_EXPECT &&
+|cat <<\EOF
+|usage: some-command [options] <args>...
+|
+|    some-command does foo and bar!
+|
+|    --hidden1             A hidden switch
+|
+|EOF
+END_EXPECT
+	test_expect_code 129 git rev-parse --parseopt -- --help-all > output < optionspec_only_hidden_switches &&
+	test_i18ncmp expect output
+'
+
+test_expect_failure 'test --parseopt invalid switch help output' '
+	sed -e "s/^|//" >expect <<\END_EXPECT &&
+|error: unknown option `does-not-exist'\''
+|usage: some-command [options] <args>...
+|
+|    some-command does foo and bar!
+|
+|    -h, --help            show the help
+|    --foo                 some nifty option --foo
+|    --bar ...             some cool option --bar with an argument
+|    -b, --baz             a short and long option
+|
+|An option group Header
+|    -C[...]               option C with an optional argument
+|    -d, --data[=...]      short and long option with an optional argument
+|
+|Argument hints
+|    -B <arg>              short option required argument
+|    --bar2 <arg>          long option required argument
+|    -e, --fuz <with-space>
+|                          short and long option required argument
+|    -s[<some>]            short option optional argument
+|    --long[=<data>]       long option optional argument
+|    -g, --fluf[=<path>]   short and long option optional argument
+|    --longest <very-long-argument-hint>
+|                          a very long argument hint
+|    --pair <key=value>    with an equals sign in the hint
+|    --aswitch             help te=t contains? fl*g characters!`
+|    --bswitch <hint>      hint has trailing tab character
+|    --cswitch             switch has trailing tab character
+|    --short-hint <a>      with a one symbol hint
+|
+|Extras
+|    --extra1              line above used to cause a segfault but no longer does
+|
+END_EXPECT
+	test_expect_code 129 git rev-parse --parseopt -- --does-not-exist 1>/dev/null 2>output < optionspec &&
+	test_i18ncmp expect output
+'
+
 test_expect_success 'setup expect.1' "
 	cat > expect <<EOF
 set -- --foo --bar 'ham' -b --aswitch -- 'arg'