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@xxxxxxxx> Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx> --- 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