> On 03 Nov 2016, at 21:12, Johannes Sixt <j...@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 <j...@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 <j...@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

Reply via email to