Re: [PATCH v11 13/14] convert: add filter.<driver>.process option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 2 Nov 2016, at 18:03, Johannes Sixt <j6t@xxxxxxxx> wrote:
> 
>> Am 17.10.2016 um 01:20 schrieb larsxschneider@xxxxxxxxx:
>> +# Compare two files and ensure that `clean` and `smudge` respectively are
>> +# called at least once if specified in the `expect` file. The actual
>> +# invocation count is not relevant because their number can vary.
>> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxx/
>> +test_cmp_count () {
>> +    expect=$1
>> +    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" &&
> 
> This is not sufficiently portable. Some versions of uniq write the
> count left-adjusted, not right-adjusted. How about this on top:
> 
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index a20b9f58e3..f60858c517 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" &&

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]+' ?

> +        mv "$FILE.tmp" "$FILE" || return

Why '|| return' here?

>    done &&
>    test_cmp "$expect" "$actual"
> }

Thank you,
Lars



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]