On Thu, Apr 21, 2016 at 5:33 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Thu, Apr 21, 2016 at 3:19 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>> On Thu, Apr 21, 2016 at 2:20 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>>> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>>>> + strbuf_addstr(&git_dir, absolute_path(get_git_dir()));
>>>> + for (i = 0; i < counter; i++) {
>>>> + struct worktree *wt = list[i];
>>>> + strbuf_addstr(&path, absolute_path(get_worktree_git_dir(wt)));
>>>> + wt->is_current = !strcmp_icase(git_dir.buf, path.buf);
>>>
>>> Can you talk a bit about why this uses 'icase'? Should it be
>>> respecting cache.h:ignore_case?
>>
>> It does.That function (in dir.c) is just one-liner
>>
>> return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
>>
>> I admit though, the naming does not make that clear.
Ugh, this is only the fourth patch, yet the second stupid review
mistake I've made thus far in this series. For some reason, I kept
reading this as a call to strcasecmp() or stricmp() rather than
strcmp_icase(). Worse, I had even consulted path.c:strcmp_icase() to
see how the issue was handled there.
> While we're at it, how about renaming it to pathcmp (and its friend
> strncmp_icase to pathncmp)?
Yes, that seems like a good idea. For anyone familiar with
strcasecmp() or stricmp(), having "icase" in the name makes it seem as
though it's unconditionally case-insensitive, so dropping it from the
name would likely be beneficial.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html