Re: [PATCH v3 2/6] worktree.c: find_worktree() learns to identify worktrees by basename

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2016, 18:44 UTC
Message-ID: <xmqqr3cgycjl.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACsJy8CmdTapWsst-PuwFNH8Uy3Vgow+fKWzQ+tGYPSc=aZsXg@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 11 quoted lines

>> That is, wouldn't you rather see "worktree lock to/foo" work when
>> 'foo' is ambiguous but 'to/foo' is not?
>
> I don't know. I suppose if people have to make `basename $path` the
> same because of some weird build settings, e.g. abc/git and def/git,
> then this basename selection becomes useless.
>
> I had similar thought though, if you only have a worktree named "foo"
> then "fo" or even "f" should be unambiguous and can also identify a
> worktree, similar to short sha-1. But I discarded that idea because of
> the higher chances that typos can select a wrong worktree.

I do not think abbreviation of "foo" down to "fo" is sensible at all. I would instead suggest "path-component-wise tail match", i.e. "to/foo" would match "path/to/foo", "path/TO/FOO" (on fold-case filesystems), but not "pathto/foo".

> We would
> need to convert or match both '/' and '\' in "to/foo" case because of
> Windows, so it's not much easier than basename().

I never said "easier to implement". But can this codepath get backslashed paths in the first place? I somehow thought that normalization would happen a lot before the control reaches here.

You'll be calling into fspathcmp() anyway; shouldn't the function know that '/' and '\' are equivalent on some platforms, or is it legal to only call fspathcmp() on a single path component without directory separator?