Re: [PATCH 1/1] contrib: add coverage-diff script
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 13, 2018, 14:59 UTC
- Message-ID
- <xmqqin39igt2.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <1e712541-3101-049a-acd2-b6d7c98bd1e2@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
Show 25 quoted lines
> On 9/12/2018 6:54 PM, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>>
>>>> contrib/coverage-diff.sh | 70 ++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 70 insertions(+)
>>>> create mode 100755 contrib/coverage-diff.sh
>>> I fully appreciate the motivation. But it is a bit sad that this
>>> begins with "#!/bin/bash" but it seems that the script is full of
>>> bash-isms. I haven't gone through the script to see if these are
>>> inevitable or gratuitous yet, but I'd assume it made it easier for
>>> you to write it to step outside the pure POSIX shell?
>
> I completely forgot to avoid bash, as I wrote this first as an experiment.
>
>>> ...
>>>> + elif [[ $REPLY =~ \+\+\+\ (b/)?([^[:blank:]$esc]+).* ]]; then
>>>> + path=${BASH_REMATCH[2]}
>>> OK, it probably is easier to write in bash than using expr if you
>>> want to do regexp.
>> Just to clarify. I am saying that it is OK to give up writing in
>> pure POSIX and relying on bash-isms after seeing these lines.
>
> I'll try rewriting it using POSIX shell and see how hard it is.Thanks. Don't waste too much time on it and try to bend backwards too far, though.