Re: [PATCH v2 7/8] remote-bzr: reorganize the way 'wanted' works

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 29, 2013, 02:31 UTC
Message-ID: <51a5686ba7f2c_807b33e189951a@nysa.mail>
In-Reply-To: <7vvc632g1s.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > +    wanted = get_config('remote-bzr.branches').rstrip().split(', ')
> 
> Two minor nits and one design suggestion:
> 
>  - Why rstrip() not strip()?

The purpose of the strip is to remove the _single_ "\n" at the end that subprocess communicate. Maybe get_config() should do that.

Show 8 quoted lines

> It appears that this only is helping
>    an end-user "mistake" like this:
> 
> 	git config remote-bzr.branches 'trunk, devel, test '
> 
>    without helping people who have done this:
> 
> 	git config remote-bzr.branches 'trunk,  devel, test'

No, that's tnot it.

Show 9 quoted lines

>  - Is
> 
>      git config remote-bzr.branches trunk,devel,test
> 
>    a grave sin?
> 
>    In other words, wouldn't we want something like this instead?
> 
> 	map(lambda s: s.strip(), get_config('...').split(','))

Yeah, that might make sense.

Show 9 quoted lines

>  - Doesn't allowing multi-valued variable, e.g.
> 
> 	[remote-bzr]
>             branches = trunk
>             branches = devel
>             branches = test
> 
>    make it easier for the user to manage this configuration by
>    e.g. selectively removing or adding tracked branches?

How would the 'git config' command look like?

Either way, that's orthogonal to this patch.

-- 
Felipe Contreras