Re: [PATCH] fetch: prefer suffix substitution in compact fetch.output

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 26, 2019, 23:57 UTC
Message-ID: <CACsJy8CzKoq41o09vNsQ=56mAtND_EaHxYN1VgVeLVTo=U2TUA@mail.gmail.com>
In-Reply-To: <xmqqef90fmi6.fsf@gitster-ct.c.googlers.com>

On Sat, Jan 26, 2019 at 4:36 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

>
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> > I have a remote named "jch" and it has a branch with the same name. And
> > fetch.output is set to "compact". Fetching this remote looks like this
> >
> >  From https://github.com/gitster/git
> >   + eb7fd39f6b...835363af2f jch                -> */jch  (forced update)
> >     6f11fd5edb..59b12ae96a  nd/config-move-to  -> jch/*
> >   * [new branch]            nd/diff-parseopt   -> jch/*
> >   * [new branch]            nd/the-index-final -> jch/*
> >
> > Notice that the local side of branch jch starts with "*" instead of
> > ending with it like the rest. It's not exactly wrong. It just looks
> > weird.
> >
> > This patch changes the find-and-replace code a bit to try finding prefix
> > first before falling back to strstr() which finds a substring from left
> > to right. Now we have something less OCD
> >
> >  From https://github.com/gitster/git
> >   + eb7fd39f6b...835363af2f jch                -> jch/*  (forced update)
> >     6f11fd5edb..59b12ae96a  nd/config-move-to  -> jch/*
> >   * [new branch]            nd/diff-parseopt   -> jch/*
> >   * [new branch]            nd/the-index-final -> jch/*
>
> Sounds good.  I do not think strstr() would ever be correct in this
> application in the first place.  In what situation would it produce
> a reasonable result, I wonder?

I think it's useful for github pull requests. The remote side is usually pull/<id>/head but when mapping to a local ref I think we often don't want a ref ending with "head", just pull/<id>. In this case, strstr() can pick the middle part and substitute it with "*".

-- 
Duy