From: René Scharfe <l.s.r@web.de>
Date: Wed, 28 Mar 2018 22:24:49 GMT
Subject: Re: [PATCH] submodule: check for NULL return of get_submodule_ref_store()
Message-ID: <096248bf-0a33-7360-e82b-b7a36e362604@web.de>
In-Reply-To: <CAGZ79kYL55+=9UbW7H-c7OUg6CHbnzdkxs0ZMsFnEAaoVXRwLw@mail.gmail.com>

Am 28.03.2018 um 23:37 schrieb Stefan Beller:
>> 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 &&

[:xdigit:] would match the hash more precisely, but I don't know how
widely this character class is supported among sed implementations.
And being so strict may require changes once SHA1 is replaced -- new
hashes might be marked with a special character.  And it's longer
anyway.

\S could be used instead, but I also don't know how widely this is
supported.

. could be used as well, of course, but that feels a bit sloppy to
me.

Using separate s commands for beginning and end of the line would
be shorter.  This here is slightly longer, anyway:

	/ repo / {s/^ /-/; s/ (.*//}

What do you have in mind?  How to transform this:

 b2613938d15a18d9d4a504cacd9654fd1c879197 repo (heads/master)

... into this:

-b2613938d15a18d9d4a504cacd9654fd1c879197 repo

... while keeping other lines unchanged?

René