On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy <pclo...@gmail.com> wrote:
> The "submodule" argument in this function is a path, which can have
> either '/' or '\\' as a separator. Use is_dir_sep() to support both.
>
> Noticed-by: Johannes Sixt <j...@kdbg.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>

Immediate questions that come to mind:
* Could this go in as an independent bug fix?
  -> If so do we have any test that fails or stops failing on Windows?
  -> If not, do we need one?
* Assuming this is not an independent bug fix:
  Why do we need this patch in this series here?
  (I thought this is about worktrees, not submodules.
  So does this fix a potential bug that will be exposed
  later in this series, but was harmless up to now?)

I'll read the next patches to see if I will be enlightened.

Thanks,
Stefan

> ---
>  refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index 3d549a8970..dec899a57a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1507,7 +1507,7 @@ int resolve_gitlink_ref(const char *submodule, const 
> char *refname,
>         struct ref_store *refs;
>         int flags;
>
> -       while (len && submodule[len - 1] == '/')
> +       while (len && is_dir_sep(submodule[len - 1]))
>                 len--;
>
>         if (!len)
> --
> 2.11.0.157.gd943d85
>

Reply via email to