* [PATCH] test-lib: simplify '--option=value' parsing
@ 2016-04-22 15:38 SZEDER Gábor
2016-04-22 18:37 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: SZEDER Gábor @ 2016-04-22 15:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor
To get the 'value' from '--option=value', test-lib.sh parses said
option running 'expr' with a regexp. This involves a subshell, an
external process, and a lot of non-alphanumeric characters in the
regexp.
Use a much simpler shell parameter expansion instead to do the same.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
t/test-lib.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6bb299..8373d5b5c900 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -208,7 +208,7 @@ do
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
--verbose-only=*)
- verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ verbose_only=${1#--*=}
shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
@@ -222,15 +222,15 @@ do
valgrind=memcheck
shift ;;
--valgrind=*)
- valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ valgrind=${1#--*=}
shift ;;
--valgrind-only=*)
- valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ valgrind_only=${1#--*=}
shift ;;
--tee)
shift ;; # was handled already
--root=*)
- root=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ root=${1#--*=}
shift ;;
--chain-lint)
GIT_TEST_CHAIN_LINT=1
--
2.8.1.99.g5d5236f
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] test-lib: simplify '--option=value' parsing
2016-04-22 15:38 [PATCH] test-lib: simplify '--option=value' parsing SZEDER Gábor
@ 2016-04-22 18:37 ` Jeff King
2016-04-22 20:32 ` [PATCH v2] " SZEDER Gábor
2016-04-22 20:54 ` [PATCH] " Junio C Hamano
0 siblings, 2 replies; 4+ messages in thread
From: Jeff King @ 2016-04-22 18:37 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, git
On Fri, Apr 22, 2016 at 05:38:23PM +0200, SZEDER Gábor wrote:
> To get the 'value' from '--option=value', test-lib.sh parses said
> option running 'expr' with a regexp. This involves a subshell, an
> external process, and a lot of non-alphanumeric characters in the
> regexp.
>
> Use a much simpler shell parameter expansion instead to do the same.
Looks OK to me. I doubt the extra process is a killer (we only parse
options once per script), but the result is definitely more readable,
IMHO.
For some reason I had always assumed that complicated pattern matching
with "#" was non-POSIX, but I checked and it is definitely in there.
> ---
> t/test-lib.sh | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
I count 5 cases in my copy of test-lib.sh. I think you are missing
"--run".
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] test-lib: simplify '--option=value' parsing
2016-04-22 18:37 ` Jeff King
@ 2016-04-22 20:32 ` SZEDER Gábor
2016-04-22 20:54 ` [PATCH] " Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: SZEDER Gábor @ 2016-04-22 20:32 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git, SZEDER Gábor
To get the 'value' from '--option=value', test-lib.sh parses said
option running 'expr' with a regexp. This involves a subshell, an
external process, and a lot of non-alphanumeric characters in the
regexp.
Use a much simpler POSIX-defined shell parameter expansion instead to
do the same.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> I count 5 cases in my copy of test-lib.sh. I think you are missing
> "--run".
Oh, indeed, '--run' managed to hide between these
-l|--l|--lo|--lon|--long patterns.
t/test-lib.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b47eb6bb299..79afa8748eec 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -202,13 +202,13 @@ do
}
run_list=$1; shift ;;
--run=*)
- run_list=$(expr "z$1" : 'z[^=]*=\(.*\)'); shift ;;
+ run_list=${1#--*=}; shift ;;
-h|--h|--he|--hel|--help)
help=t; shift ;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
--verbose-only=*)
- verbose_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ verbose_only=${1#--*=}
shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
@@ -222,15 +222,15 @@ do
valgrind=memcheck
shift ;;
--valgrind=*)
- valgrind=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ valgrind=${1#--*=}
shift ;;
--valgrind-only=*)
- valgrind_only=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ valgrind_only=${1#--*=}
shift ;;
--tee)
shift ;; # was handled already
--root=*)
- root=$(expr "z$1" : 'z[^=]*=\(.*\)')
+ root=${1#--*=}
shift ;;
--chain-lint)
GIT_TEST_CHAIN_LINT=1
--
2.8.1.99.g5d5236f
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] test-lib: simplify '--option=value' parsing
2016-04-22 18:37 ` Jeff King
2016-04-22 20:32 ` [PATCH v2] " SZEDER Gábor
@ 2016-04-22 20:54 ` Junio C Hamano
1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2016-04-22 20:54 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, git
Jeff King <peff@peff.net> writes:
> For some reason I had always assumed that complicated pattern matching
> with "#" was non-POSIX, but I checked and it is definitely in there.
No, the offending code really dates back to the days back when I was
writing most of the shell scripts (or others were mostly copying
from what I have written). I had this irrational aversion against
variable substitutions that are beyond ${var-default}, ${var=assign}
and ${var+isset}.
The conversion looks fine (modulo one missed one you noticed).
Thanks for cleaning things up.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-22 20:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-22 15:38 [PATCH] test-lib: simplify '--option=value' parsing SZEDER Gábor
2016-04-22 18:37 ` Jeff King
2016-04-22 20:32 ` [PATCH v2] " SZEDER Gábor
2016-04-22 20:54 ` [PATCH] " Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.