Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> On Thu, Feb 24 2022, Glen Choo wrote:
>
>> +check_sub() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.sub
>
> Hrm, I didn't know that would work, the usual style is:
>
> cat >file <<...
>
> Instead of:
>
> cat <<.. >file
>
> Maybe better to use that?
Thanks, I somehow mixed things up when I wrote that.
>> + Fetching submodule submodule
>> + From $pwd/submodule
>> + OLD_HEAD..$NEW_HEAD sub -> origin/sub
>> + EOF
>> +}
>> +
>> +check_deep() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.deep
>> + Fetching submodule submodule/subdir/deepsubmodule
>> + From $pwd/deepsubmodule
>> + OLD_HEAD..$NEW_HEAD deep -> origin/deep
>> + EOF
>> +}
>> +
>> +check_super() {
>> + NEW_HEAD=$1 &&
>> + cat <<-EOF >$pwd/expect.err.super
>> + From $pwd/.
>> + OLD_HEAD..$NEW_HEAD super -> origin/super
>> + EOF
>> +}
>
> These look a lot better, but instead of always passing the result of
> "git rev-parse --short HEAD" can't we just always invoke that in these
> helpers?
>
> Maybe there are cases where $NEW_HEAD is different, I've just skimmed
> this series.
I haven't found any other instances where $NEW_HEAD is different, so I
suppose we could move it into the helpers. I don't think it benefits
readability that much to do so, but if you think it's much better, I'll
incorporate it when I reroll this.
>> @@ -62,7 +82,8 @@ verify_fetch_result() {
>> if [ -f expect.err.deep ]; then
>> cat expect.err.deep >>expect.err.combined
>> fi &&
>> - test_cmp expect.err.combined $ACTUAL_ERR
>> + sed -E 's/[0-9a-f]+\.\./OLD_HEAD\.\./' $ACTUAL_ERR >actual.err.cmp &&
>> + test_cmp expect.err.combined actual.err.cmp
>> }
>
> I think this is unportable per check-non-portable-shell.pl:
>
> /\bsed\s+-[^efn]\s+/ and err 'sed option not portable (use only -n, -e, -f)';
Ah thanks, my sed-fu is pretty poor, so I appreciate the tip :)
I used that because I wanted +, but I found what I needed from the sed
manpage i.e. that + is equivalent to \{1,\}).
