On Fri, Feb 18 2022, John Cai via GitGitGadget wrote:
> From: John Cai <johncai86@xxxxxxxxx>
>
> 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 <johncai86@xxxxxxxxx>
> ---
> 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...
