git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matheus Tavares <matheus.bernardino@usp.br>
To: gitster@pobox.com
Cc: git@vger.kernel.org, Johannes.Schindelin@gmx.de, j6t@kdbg.org,
	peff@peff.net, szeder.dev@gmail.com
Subject: [PATCH v3] test-lib: allow short options to be bundled
Date: Sun, 22 Mar 2020 12:58:57 -0300	[thread overview]
Message-ID: <cec4bbcc4270914a729a2c65d9c0efc4a45742ce.1584854619.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <7a6a8197dcd58e8690892d03cb904dd1eec5d7c1.1584818457.git.matheus.bernardino@usp.br>

When debugging a test (or a set of tests), it's common to execute it
with some combination of short options, such as:

	$ ./txxx-testname.sh -d -x -i

In cases like this, CLIs usually allow the short options to be bundled
in a single argument, for convenience and agility. Let's add this
feature to test-lib, allowing the above command to be run as:

	$ ./txxx-testname.sh -dxi
	(or any other permutation, e.g. '-ixd')

Note: Short options that require an argument can also be used in a
bundle, in any position. So, for example, '-r 5 -x', '-xr 5' and '-rx 5'
are all valid and equivalent. A special case would be having a bundle
with more than one of such options. To keep things simple, this case is
not allowed for now. This shouldn't be a major limitation, though, as
the only short option that requires an argument today is '-r'. And
concatenating '-r's as in '-rr 5 6' would probably not be very
practical: its unbundled format would be '-r 5 -r 6', for which test-lib
currently considers only the last argument. Therefore, if '-rr 5 6' were
to be allowed, it would have the same effect as just typing '-r 6'.

Note: the test-lib currently doesn't support '-r5', as an alternative
for '-r 5', so the former is not supported in bundles as well.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---

Changes since v2:
- Fixed style in mark_option_requires_arg(), moving "{" to the same line
- In the main options parser loop, joined long options and unbundled
  short options in the same case arm, as suggested by J6t. This provides
  a clearer codeflow.
- Summarized, in the commit message, the approach chosen on bundling
  options that require args. Since this was an important point in the
  last rounds, I thought it would be good to include our decisions (and
  justification) in the commit body.

 t/README      |  3 ++-
 t/test-lib.sh | 61 +++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/t/README b/t/README
index 9afd61e3ca..080bc59fc4 100644
--- a/t/README
+++ b/t/README
@@ -69,7 +69,8 @@ You can also run each test individually from command line, like this:
 
 You can pass --verbose (or -v), --debug (or -d), and --immediate
 (or -i) command line argument to the test, or by setting GIT_TEST_OPTS
-appropriately before running "make".
+appropriately before running "make". Short options can be bundled, i.e.
+'-d -v' is the same as '-dv'.
 
 -v::
 --verbose::
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0ea1e5a05e..a6178e9fac 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -78,20 +78,23 @@ then
 	exit 1
 fi
 
-# Parse options while taking care to leave $@ intact, so we will still
-# have all the original command line options when executing the test
-# script again for '--tee' and '--verbose-log' below.
 store_arg_to=
-prev_opt=
-for opt
-do
-	if test -n "$store_arg_to"
+opt_required_arg=
+# $1: option string
+# $2: name of the var where the arg will be stored
+mark_option_requires_arg () {
+	if test -n "$opt_required_arg"
 	then
-		eval $store_arg_to=\$opt
-		store_arg_to=
-		prev_opt=
-		continue
+		echo "error: options that require args cannot be bundled" \
+			"together: '$opt_required_arg' and '$1'" >&2
+		exit 1
 	fi
+	opt_required_arg=$1
+	store_arg_to=$2
+}
+
+parse_option () {
+	local opt="$1"
 
 	case "$opt" in
 	-d|--d|--de|--deb|--debu|--debug)
@@ -101,7 +104,7 @@ do
 	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
 		GIT_TEST_LONG=t; export GIT_TEST_LONG ;;
 	-r)
-		store_arg_to=run_list
+		mark_option_requires_arg "$opt" run_list
 		;;
 	--run=*)
 		run_list=${opt#--*=} ;;
@@ -185,12 +188,42 @@ do
 	*)
 		echo "error: unknown test option '$opt'" >&2; exit 1 ;;
 	esac
+}
+
+# Parse options while taking care to leave $@ intact, so we will still
+# have all the original command line options when executing the test
+# script again for '--tee' and '--verbose-log' later.
+for opt
+do
+	if test -n "$store_arg_to"
+	then
+		eval $store_arg_to=\$opt
+		store_arg_to=
+		opt_required_arg=
+		continue
+	fi
 
-	prev_opt=$opt
+	case "$opt" in
+	--*|-?)
+		parse_option "$opt" ;;
+	-?*)
+		# bundled short options must be fed separately to parse_option
+		opt=${opt#-}
+		while test -n "$opt"
+		do
+			extra=${opt#?}
+			this=${opt%$extra}
+			opt=$extra
+			parse_option "-$this"
+		done
+		;;
+	*)
+		echo "error: unknown test option '$opt'" >&2; exit 1 ;;
+	esac
 done
 if test -n "$store_arg_to"
 then
-	echo "error: $prev_opt requires an argument" >&2
+	echo "error: $opt_required_arg requires an argument" >&2
 	exit 1
 fi
 
-- 
2.25.1


  parent reply	other threads:[~2020-03-22 15:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-21  3:07 [PATCH] test-lib: allow short options to be stacked Matheus Tavares
2020-03-21  4:53 ` Junio C Hamano
2020-03-21 17:27   ` Matheus Tavares
2020-03-21  6:26 ` Jeff King
2020-03-21 18:50   ` Matheus Tavares Bernardino
2020-03-22  6:49     ` Jeff King
2020-03-22  8:14     ` SZEDER Gábor
2020-03-21 18:57   ` Junio C Hamano
2020-03-21  8:55 ` Johannes Sixt
2020-03-21 18:55   ` Matheus Tavares Bernardino
2020-03-21 20:11     ` Junio C Hamano
2020-03-21 19:57 ` [PATCH v2] test-lib: allow short options to be bundled Matheus Tavares
2020-03-21 20:07   ` Junio C Hamano
2020-03-21 22:42     ` Matheus Tavares Bernardino
2020-03-22 15:58   ` Matheus Tavares [this message]
2020-03-23 20:18     ` [PATCH v3] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cec4bbcc4270914a729a2c65d9c0efc4a45742ce.1584854619.git.matheus.bernardino@usp.br \
    --to=matheus.bernardino@usp.br \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=peff@peff.net \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).