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

Signed-off-by: David Bryson <redacted>

I tried to keep with the naming/coding conventions that I found in
remote.c.  Feedback welcome.

---
 remote.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/remote.c b/remote.c
index 3f3c789..893a739 100644
--- a/remote.c
+++ b/remote.c

@@ -305,6 +305,7 @@ static int handle_config(const char *key, const char *value, void *cb)
 {
 	const char *name;
 	const char *subkey;
+	const char *v;
 	struct remote *remote;
 	struct branch *branch;
 	if (!prefixcmp(key, "branch.")) {

@@ -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;
 			if (branch == current_branch)
 				default_remote_name = branch->remote_name;
 		} else if (!strcmp(subkey, ".merge")) {
-			if (!value)
-				return config_error_nonbool(key);
-			add_merge(branch, xstrdup(value));
+			if (git_config_string(&v, key, value )) 
+				return 	-1;
+			add_merge(branch, v);
 		}
 		return 0;
 	}

@@ -334,9 +335,9 @@ static int handle_config(const char *key, const char *value, void *cb)
 			return 0;
 		rewrite = make_rewrite(name, subkey - name);
 		if (!strcmp(subkey, ".insteadof")) {
-			if (!value)
-				return config_error_nonbool(key);
-			add_instead_of(rewrite, xstrdup(value));
+			if (git_config_string(&v, key, value )) 
+				return 	-1;
+			add_instead_of(rewrite, v);
 		}
 	}
 	if (prefixcmp(key,  "remote."))

-- 
1.6.0.2

David Bryson wrote:

Not very mnemonic. I'm sure you can think up a better name, even if it's
a long one. Git is notoriously sparse when it comes to comments. We rely
instead on self-explanatory code.

Other than that, the patch looks good.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

On Fri, Oct 03, 2008 at 07:28:42AM +0200 or thereabouts, Andreas Ericsson wrote:

David Bryson wrote:

quoted

Signed-off-by: David Bryson <redacted>
I tried to keep with the naming/coding conventions that I found in
remote.c.  Feedback welcome.
---
 remote.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/remote.c b/remote.c
index 3f3c789..893a739 100644
--- a/remote.c
+++ b/remote.c

@@ -305,6 +305,7 @@ static int handle_config(const char *key, const char 

*value, void *cb)
 {
 	const char *name;
 	const char *subkey;
+	const char *v;

Not very mnemonic. I'm sure you can think up a better name, even if it's
a long one. Git is notoriously sparse when it comes to comments. We rely
instead on self-explanatory code.

Oh I agree entirely, it is quite vague, however like I mentioned I tried
to keep to the conventios in the file.  This strategy(v) is used in several
other places in remote.c, if this is Bad Code, then I have no problem
changing it.

Thoughts from anybody else ?

2008/10/3 David Bryson [off-list ref]:

On Fri, Oct 03, 2008 at 07:28:42AM +0200 or thereabouts, Andreas Ericsson wrote:

quoted

David Bryson wrote:

Oh I agree entirely, it is quite vague, however like I mentioned I tried
to keep to the conventios in the file.  This strategy(v) is used in several
other places in remote.c, if this is Bad Code, then I have no problem
changing it.

Thoughts from anybody else ?

You can redeclare of the variable in the contexts where
it is used and not even rename it: it is close to its users then.

Hi,

On Thu, 2 Oct 2008, David Bryson wrote:

Signed-off-by: David Bryson <redacted>

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.

What is the reason not to write

			if (git_config_string(&branch->remote_name, key, value))
				return -1;

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

Ciao,
Dscho

On Sun, Oct 05, 2008 at 12:46:29AM +0200 or thereabouts, Alex Riesen wrote:

quoted

You can redeclare of the variable in the contexts where
it is used and not even rename it: it is close to its users then.

I'm not sure I understand entirely, care to elaborate a bit more ?

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:

quoted

Signed-off-by: David Bryson <redacted>

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.

quoted

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