Re: [PATCH v2 4/4] builtin/branch: strip refs/heads/ using skip_prefix

2017-11-22 Thread Kaartic Sivaraam
On Wednesday 22 November 2017 07:39 AM, Junio C Hamano wrote: Eric Sunshine writes: On Tue, Nov 21, 2017 at 2:12 PM, Kaartic Sivaraam wrote: On Wednesday 22 November 2017 12:08 AM, Eric Sunshine wrote: The original code unconditionally

Re: [PATCH v2 4/4] builtin/branch: strip refs/heads/ using skip_prefix

2017-11-21 Thread Junio C Hamano
Eric Sunshine writes: > On Tue, Nov 21, 2017 at 2:12 PM, Kaartic Sivaraam > wrote: >> On Wednesday 22 November 2017 12:08 AM, Eric Sunshine wrote: >>> The original code unconditionally uses "+ 11", which says that the >>> prefix is _always_

Re: [PATCH v2 4/4] builtin/branch: strip refs/heads/ using skip_prefix

2017-11-21 Thread Eric Sunshine
On Tue, Nov 21, 2017 at 2:12 PM, Kaartic Sivaraam wrote: > On Wednesday 22 November 2017 12:08 AM, Eric Sunshine wrote: >> The original code unconditionally uses "+ 11", which says that the >> prefix is _always_ present. This commit message muddies the waters [...] > >

Re: [PATCH v2 4/4] builtin/branch: strip refs/heads/ using skip_prefix

2017-11-21 Thread Kaartic Sivaraam
On Wednesday 22 November 2017 12:08 AM, Eric Sunshine wrote: On Tue, Nov 21, 2017 at 9:18 AM, Kaartic Sivaraam wrote: Though we don't check for the result of verification here as it's (almost) always the case that the string does start with "refs/heads", it's just

Re: [PATCH v2 4/4] builtin/branch: strip refs/heads/ using skip_prefix

2017-11-21 Thread Eric Sunshine
On Tue, Nov 21, 2017 at 9:18 AM, Kaartic Sivaraam wrote: > Instead of hard-coding the offset strlen("refs/heads/") to skip > the prefix "refs/heads/" use the skip_prefix() function which > is more communicative and verifies that the string actually > starts with that