On Fri, Dec 9, 2016 at 7:28 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Brandon Williams <bmwill@xxxxxxxxxx> writes:
>
>> +static void strip_submodule_slash_cheap(struct pathspec_item *item)
>> +{
>> + int i;
>> +
>> + if ((item->len >= 1 && item->match[item->len - 1] == '/') &&
>> + (i = cache_name_pos(item->match, item->len - 1)) >= 0 &&
>> + S_ISGITLINK(active_cache[i]->ce_mode)) {
>> + item->len--;
>> + item->match[item->len] = '\0';
>> + }
>> +}
>
> I know that this is merely a moved code, but while I was reading
> this, it triggered "Do not make an assignment inside if condition"
> check.
Yeah with a function of its own, it's probably better to separate that
assignment.
> But more importantly, is the code even correct? If the path
> for the submodule is unmerged, we would get a negative i that points
> at the conflicting entry; don't we want to do something about it, at
> least when we have a submodule entry at stage #2 (i.e. ours)?
In my defense I was simply moving (again!) the code from
strip_trailing_slash_from_submodules() in b69bb3f:builtin/ls-files.c.
Could be an improvement point for submodule people, I guess.
--
Duy