From: Junio C Hamano Date: Fri, 19 Feb 2016 03:14:42 GMT Subject: Re: [PATCHv12 0/7] Expose submodule parallelism to the user Message-ID: In-Reply-To: Stefan Beller writes: > On Thu, Feb 18, 2016 at 3:20 PM, Junio C Hamano wrote: >> Stefan Beller writes: >> >>> On Thu, Feb 18, 2016 at 3:12 PM, Stefan Beller 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;