Re: [PATCH v10 3/4] cat-file: add remove_timestamp helper
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-02-19 06:35:10
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Fri, Feb 18 2022, John Cai via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: John Cai <redacted> maybe_remove_timestamp() takes arguments, but it would be useful to have a function that reads from stdin and strips the timestamp. This would allow tests to pipe data into a function to remove timestamps, and wouldn't have to always assign a variable. This is especially helpful when the data is multiple lines. Keep maybe_remove_timestamp() the same, but add a remove_timestamp helper that reads from stdin. The tests in the next patch will make use of this. Signed-off-by: John Cai <redacted> --- t/t1006-cat-file.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 145eee11df9..2d52851dadc 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh@@ -105,13 +105,18 @@ strlen () { } maybe_remove_timestamp () { - if test -z "$2"; then - echo_without_newline "$1" - else - echo_without_newline "$(printf '%s\n' "$1" | sed -e 's/ [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$//')" - fi + if test -z "$2"; then + echo_without_newline "$1" + else + echo_without_newline "$(printf '%s\n' "$1" | remove_timestamp)" + fi } +remove_timestamp () { + sed -e 's/ [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$//' +} + + run_tests () { type=$1 sha1=$2
I may have missed some previous discussions, but is there a reason this echo_without_newline dance is needed? At this point this on top passes all tests for me on both dash and bash:
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 2d52851dadc..8266a939f99 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh@@ -104,18 +104,19 @@ strlen () { echo_without_newline "$1" | wc -c | sed -e 's/^ *//' } -maybe_remove_timestamp () { - if test -z "$2"; then - echo_without_newline "$1" - else - echo_without_newline "$(printf '%s\n' "$1" | remove_timestamp)" - fi -} - remove_timestamp () { sed -e 's/ [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$//' } +maybe_remove_timestamp () { + if test -n "$2" + then + echo "$1" | remove_timestamp + return 0 + fi + + echo "$1" +} run_tests () { type=$1
The move is another comment, if we're adding a remove_timestamp() let's define it before maybe_remove_timestamp() which uses it, even though in this case we can get away with it...