Re: [PATCHv12 0/7] Expose submodule parallelism to the user

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 19, 2016, 03:14 UTC
Message-ID: <xmqqbn7de6hp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kY7aYmDcz+JYwpzizfzRV3bUeN8TU-4jRjKqToJJTBHoA@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 16 quoted lines

> On Thu, Feb 18, 2016 at 3:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> On Thu, Feb 18, 2016 at 3:12 PM, Stefan Beller <sbeller@google.com> wrote:
>>>>> Unless you count "I want to write differently from what was
>>>>> suggested" is a desirable thing to do, I do not see a point in
>>>>> favouring the above that uses an extra variable and skip_prefix()
>>>>> over what I gave you as "how about" patch.  But whatever.
>>>>
>>>> The skip_prefix was there before, so it stuck there.
>>
>> Sorry, but I thought this "parsing update strategy" was all new
>> code.
>
> I meant previous patches or in my mind. That's why I was hesitant to
> throw out the skip_prefix.

I actually think the attached on top of your final version would be the best. It would not make too big a difference in this codepath that skips just one byte, the pattern naturally would apply to prefix of any length, and this would serve as the BCP, ready to be copied-and-pasted by others when writing new code.

And of course it does not waste an otherwise unnecessary temporary variable ;-)

diff --git a/submodule.c b/submodule.c
index 911fa3b..8e08159 100644
--- a/submodule.c
+++ b/submodule.c
@@ -223,9 +223,9 @@ int parse_submodule_update_strategy(const char *value,
 		dst->type = SM_UPDATE_REBASE;
 	else if (!strcmp(value, "merge"))
 		dst->type = SM_UPDATE_MERGE;
-	else if (value[0] == '!') {
+	else if (skip_prefix(value, "!", &value)) {
 		dst->type = SM_UPDATE_COMMAND;
-		dst->command = xstrdup(value + 1);
+		dst->command = xstrdup(value);
 	} else
 		return -1;
 	return 0;