{"thread":{"id":"42110","subject":"[PATCH] test-lib: simplify '--option=value' parsing","startedAt":"2016-04-22T15:38:23Z","lastAt":"2016-04-22T20:54:55Z","messageCount":4,"participants":["SZEDER Gábor","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"284150","messageId":"1461339503-6854-1-git-send-email-szeder@ira.uka.de","threadId":"42110","inReplyTo":null,"subject":"[PATCH] test-lib: simplify '--option=value' parsing","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-04-22T15:38:23Z","receivedAt":"2016-04-22T15:38:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"To get the 'value' from '--option=value', test-lib.sh parses said\noption running 'expr' with a regexp.  This involves a subshell, an\nexternal process, and a lot of non-alphanumeric characters in the\nregexp.\n\nUse a much simpler shell parameter expansion instead to do the same.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n t/test-lib.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0b47eb6bb299..8373d5b5c900 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -208,7 +208,7 @@ do\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n \t\tverbose=t; shift ;;\n \t--verbose-only=*)\n-\t\tverbose_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tverbose_only=${1#--*=}\n \t\tshift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\t# Ignore --quiet under a TAP::Harness. Saying how many tests\n@@ -222,15 +222,15 @@ do\n \t\tvalgrind=memcheck\n \t\tshift ;;\n \t--valgrind=*)\n-\t\tvalgrind=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tvalgrind=${1#--*=}\n \t\tshift ;;\n \t--valgrind-only=*)\n-\t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tvalgrind_only=${1#--*=}\n \t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n-\t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\troot=${1#--*=}\n \t\tshift ;;\n \t--chain-lint)\n \t\tGIT_TEST_CHAIN_LINT=1\n-- \n2.8.1.99.g5d5236f\n"},{"id":"284163","messageId":"20160422183703.GA7595@sigill.intra.peff.net","threadId":"42110","inReplyTo":"1461339503-6854-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] test-lib: simplify '--option=value' parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-22T18:37:03Z","receivedAt":"2016-04-22T18:37:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 22, 2016 at 05:38:23PM +0200, SZEDER Gábor wrote:\n\n> To get the 'value' from '--option=value', test-lib.sh parses said\n> option running 'expr' with a regexp.  This involves a subshell, an\n> external process, and a lot of non-alphanumeric characters in the\n> regexp.\n> \n> Use a much simpler shell parameter expansion instead to do the same.\n\nLooks OK to me. I doubt the extra process is a killer (we only parse\noptions once per script), but the result is definitely more readable,\nIMHO.\n\nFor some reason I had always assumed that complicated pattern matching\nwith \"#\" was non-POSIX, but I checked and it is definitely in there.\n\n> ---\n>  t/test-lib.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n\nI count 5 cases in my copy of test-lib.sh. I think you are missing\n\"--run\".\n\n-Peff\n"},{"id":"284187","messageId":"1461357141-12417-1-git-send-email-szeder@ira.uka.de","threadId":"42110","inReplyTo":"20160422183703.GA7595@sigill.intra.peff.net","subject":"[PATCH v2] test-lib: simplify '--option=value' parsing","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-04-22T20:32:21Z","receivedAt":"2016-04-22T20:32:21Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"To get the 'value' from '--option=value', test-lib.sh parses said\noption running 'expr' with a regexp.  This involves a subshell, an\nexternal process, and a lot of non-alphanumeric characters in the\nregexp.\n\nUse a much simpler POSIX-defined shell parameter expansion instead to\ndo the same.\n\nSigned-off-by: SZEDER Gábor <szeder@ira.uka.de>\n---\n\n>>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> I count 5 cases in my copy of test-lib.sh. I think you are missing\n> \"--run\".\n\nOh, indeed, '--run' managed to hide between these\n-l|--l|--lo|--lon|--long patterns.\n\n\n t/test-lib.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0b47eb6bb299..79afa8748eec 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -202,13 +202,13 @@ do\n \t\t}\n \t\trun_list=$1; shift ;;\n \t--run=*)\n-\t\trun_list=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)'); shift ;;\n+\t\trun_list=${1#--*=}; shift ;;\n \t-h|--h|--he|--hel|--help)\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n \t\tverbose=t; shift ;;\n \t--verbose-only=*)\n-\t\tverbose_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tverbose_only=${1#--*=}\n \t\tshift ;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\t# Ignore --quiet under a TAP::Harness. Saying how many tests\n@@ -222,15 +222,15 @@ do\n \t\tvalgrind=memcheck\n \t\tshift ;;\n \t--valgrind=*)\n-\t\tvalgrind=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tvalgrind=${1#--*=}\n \t\tshift ;;\n \t--valgrind-only=*)\n-\t\tvalgrind_only=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\tvalgrind_only=${1#--*=}\n \t\tshift ;;\n \t--tee)\n \t\tshift ;; # was handled already\n \t--root=*)\n-\t\troot=$(expr \"z$1\" : 'z[^=]*=\\(.*\\)')\n+\t\troot=${1#--*=}\n \t\tshift ;;\n \t--chain-lint)\n \t\tGIT_TEST_CHAIN_LINT=1\n-- \n2.8.1.99.g5d5236f\n"},{"id":"284189","messageId":"xmqq1t5xpdnk.fsf@gitster.mtv.corp.google.com","threadId":"42110","inReplyTo":"20160422183703.GA7595@sigill.intra.peff.net","subject":"Re: [PATCH] test-lib: simplify '--option=value' parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-22T20:54:55Z","receivedAt":"2016-04-22T20:54:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> For some reason I had always assumed that complicated pattern matching\n> with \"#\" was non-POSIX, but I checked and it is definitely in there.\n\nNo, the offending code really dates back to the days back when I was\nwriting most of the shell scripts (or others were mostly copying\nfrom what I have written).  I had this irrational aversion against\nvariable substitutions that are beyond ${var-default}, ${var=assign}\nand ${var+isset}.\n\nThe conversion looks fine (modulo one missed one you noticed).\n\nThanks for cleaning things up.\n"}]}