Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/06/2017 02:23 AM, Junio C Hamano wrote: > Michael Haggerty writes: > >> [1] I say "almost entirely" because putting them in one function means >> that only `pattern` needs to be scanned for glob characters. But that is >> an unimportant detail. > > That could

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Rafael Ascensão
On 06/11/17 01:23, Junio C Hamano wrote: > Michael Haggerty writes: > >> [1] I say "almost entirely" because putting them in one function means >> that only `pattern` needs to be scanned for glob characters. But that is >> an unimportant detail. > > That could actually be

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Junio C Hamano
Michael Haggerty writes: > [1] I say "almost entirely" because putting them in one function means > that only `pattern` needs to be scanned for glob characters. But that is > an unimportant detail. That could actually be an important detail, in that even if prefix has

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/04/2017 01:41 AM, Rafael Ascensão wrote: > `for_each_glob_ref_in` has some code built into it that converts > partial refs like 'heads/master' to their full qualified form > 'refs/heads/master'. It also assume a trailing '/*' if no glob > characters are present in the pattern. > > Extract

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-05 Thread Michael Haggerty
On 11/04/2017 11:45 PM, Kevin Daudt wrote: > On Sat, Nov 04, 2017 at 11:27:39AM +0900, Junio C Hamano wrote: >> I however notice that addition of /* to the tail is trying to be >> careful by using strbuf_complete('/'), but prefixing with "refs/" >> does not and we would end up with a double-slash

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-04 Thread Kevin Daudt
On Sat, Nov 04, 2017 at 11:27:39AM +0900, Junio C Hamano wrote: > I however notice that addition of /* to the tail is trying to be > careful by using strbuf_complete('/'), but prefixing with "refs/" > does not and we would end up with a double-slash if pattern begins > with a slash. The contract

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-04 Thread Rafael Ascensão
On 04/11/17 02:27, Junio C Hamano wrote: Rafael Ascensão writes: Signed-off-by: Kevin Daudt Signed-off-by: Rafael Ascensão Could you explain Kevin's sign-off we see above? It is a bit unusual (I am not yet saying it is wrong---I

Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-03 Thread Junio C Hamano
Rafael Ascensão writes: > `for_each_glob_ref_in` has some code built into it that converts > partial refs like 'heads/master' to their full qualified form s/full// (read: that "full" needs "y" at the end). > 'refs/heads/master'. It also assume a trailing '/*' if no glob

[PATCH v1 1/2] refs: extract function to normalize partial refs

2017-11-03 Thread Rafael Ascensão
`for_each_glob_ref_in` has some code built into it that converts partial refs like 'heads/master' to their full qualified form 'refs/heads/master'. It also assume a trailing '/*' if no glob characters are present in the pattern. Extract that logic to its own function which can be reused elsewhere