Re: [PATCH] t0021: expect more variations in the output of uniq -c
- From
Lars Schneider <larsxschneider@gmail.com>
- Date
- Nov 6, 2016, 15:31 UTC
- Message-ID
- <0B7DE8F1-34DF-4271-8936-B9B596EF5F9E@gmail.com>
- In-Reply-To
- <fb4d62de-fbb5-a2b4-8eba-b135125dafa9@kdbg.org>
Show 52 quoted lines
> On 03 Nov 2016, at 21:12, Johannes Sixt <j6t@kdbg.org> wrote:
>
> Some versions of uniq -c write the count left-justified, other version
> write it right-justified. Be prepared for both kinds.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> Here it is as a proper patch.
>
> Am 03.11.2016 um 01:41 schrieb Lars Schneider:
>>> On 2 Nov 2016, at 18:03, Johannes Sixt <j6t@kdbg.org> wrote:
>>> + sort "$FILE" | uniq -c |
>>> + sed -e "s/^ *[0-9][0-9]* *IN: /x IN: /" >"$FILE.tmp" &&
>>
>> This looks good (thanks for cleaning up the redundant clean/smudge
>> stuff - that was a refactoring artifact!). One minor nit: doesn't sed
>> understand '[0-9]+' ?
>
> I don't think so.
>
>>> + mv "$FILE.tmp" "$FILE" || return
>>
>> Why '|| return' here?
>
> If there is an error in the pipeline or in the mv command, the for loop
> would not exit otherwise. The subsequent test_cmp most likely fails,
> but the || return is more correct.
>
> t/t0021-conversion.sh | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index a20b9f58e3..db71acacb3 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -40,10 +40,9 @@ test_cmp_count () {
> actual=$2
> for FILE in "$expect" "$actual"
> do
> - sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
> - sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
> - sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
> - mv "$FILE.tmp" "$FILE"
> + sort "$FILE" | uniq -c |
> + sed -e "s/^ *[0-9][0-9]*[ ]*IN: /x IN: /" >"$FILE.tmp" &&
> + mv "$FILE.tmp" "$FILE" || return
> done &&
> test_cmp "$expect" "$actual"
> }
> --
> 2.11.0.rc0.55.gd967357
> This looks good to me. I wonder if I should post a patch to add the "|| return" trick to the following function "test_cmp_exclude_clean", too?!
Thanks, Lars