Re: [PATCH] Use "git_config_string" to simplify "remote.c" code in "handle_config"

[David Bryson <david@statichacks.org>]

Johannes,

On Mon, Oct 06, 2008 at 04:13:17PM +0200 or thereabouts, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 2 Oct 2008, David Bryson wrote:
> 
> > 
> > Signed-off-by: David Bryson <david@statichacks.org>
> > 
> > I tried to keep with the naming/coding conventions that I found in
> > remote.c.  Feedback welcome.
> > 
> > ---
> 
> Usually this comment goes after the --- but other than that, the form is 
> as perfect as you can wish for.

I see, still trying to remember all the little tricks for proper
submission, thanks.

> > @@ -314,15 +315,15 @@ static int handle_config(const char *key, const char *value, void *cb)
> >  			return 0;
> >  		branch = make_branch(name, subkey - name);
> >  		if (!strcmp(subkey, ".remote")) {
> > -			if (!value)
> > -				return config_error_nonbool(key);
> > -			branch->remote_name = xstrdup(value);
> > +			if (git_config_string(&v, key, value) ) 
> > +				return -1;
> > +			branch->remote_name = v;
> 
> What is the reason not to write
> 
> 			if (git_config_string(&branch->remote_name, key, value))
> 				return -1;

The only reason is it did not come to mind ;-)  But it does make the
statement somewhat clearer.

> ?  (Also note that we do not like the space between the two closing 
> parentheses.)

An oversight to be sure and not intentional, I read the CodingGuidelines
very carefully ;-)

Dave