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

[Felipe Contreras <felipe.contreras@gmail.com>]

Junio C Hamano <gitster@pobox.com> wrote:
> 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.

> 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.

>  - 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.

>  - 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