Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 28, 2018, 22:35 UTC
Message-ID: <xmqqd0zn7rqx.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAGZ79kYL55+=9UbW7H-c7OUg6CHbnzdkxs0ZMsFnEAaoVXRwLw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 10 quoted lines

>> This looks nicer here in the script, but doesn't test exactly what users
>> type most of the time, I suppose.
>>
>> So how about this?
>
> Looks good to me, though I had a nagging feeling at first that the
> regex could be made more concise.
> Why do we need the optional "[^ ]" inside \1 ?
>
>> +       sed -e "s/^ \([^ ]* repo\) .*/-\1/" <actual >expect &&

At that position there's 40-hex object name. If we want to go looser, you could say

	"s/^ \(.* repo\) .*/-\1/"

and if you want to go more strict, you could say

	"s/^ \($_x40 repo\) (heads\/master)$/-\1/"

I think "Here between the leading SP and SP before the pathname 'repo', we expect an object name which should be a run of non SP bytes" is a reasonable mid-point that is stricter than "anything goes" and is still concise.