Stefan Beller <sbel...@google.com> writes:

> On Thu, Feb 18, 2016 at 3:20 PM, Junio C Hamano <gits...@pobox.com> wrote:
>> Stefan Beller <sbel...@google.com> writes:
>>
>>> On Thu, Feb 18, 2016 at 3:12 PM, Stefan Beller <sbel...@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;


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to