[PATCH] Add git-config --remove-section, document --rename-section

[PATCH] Add git-config --remove-section, document --rename-section

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

As per the subject.  The existing --rename-section option is 
undocumented, while --remove-section is added by this patch based on the 
code I had written for the "git branch" patch.

Paolo

[-- Attachment #2: git-builtin-config-removesec.patch --]
[-- Type: text/plain, Size: 6314 bytes --]

* git-config: document --rename-section, provide --remove-section

This patch documents the previously undocumented option --rename-section
and adds a new option to zap an entire section.


diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 6624484..68de588 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,6 +16,8 @@ SYNOPSIS
 'git-config' [--global] [type] --get-all name [value_regex]
 'git-config' [--global] [type] --unset name [value_regex]
 'git-config' [--global] [type] --unset-all name [value_regex]
+'git-config' [--global] [type] --rename-section old_name new_name
+'git-config' [--global] [type] --remove-section name
 'git-config' [--global] -l | --list
 
 DESCRIPTION
@@ -74,6 +76,12 @@ OPTIONS
 --global::
 	Use global ~/.gitconfig file rather than the repository .git/config.
 
+--remove-section::
+	Remove the given section from the configuration file.
+
+--rename-section::
+	Rename the given section to a new name.
+
 --unset::
 	Remove the line matching the key from config file.
 
diff --git a/builtin-config.c b/builtin-config.c
index f1433a4..6b12fa1 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -2,7 +2,7 @@
 #include "cache.h"
 
 static const char git_config_set_usage[] =
-"git-config [ --global ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --list";
+"git-config [ --global ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list";
 
 static char *key;
 static regex_t *key_regexp;
@@ -168,6 +168,19 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			}
 			return 0;
 		}
+		else if (!strcmp(argv[1], "--remove-section")) {
+			int ret;
+			if (argc != 3)
+				usage(git_config_set_usage);
+			ret = git_config_remove_section(argv[2]);
+			if (ret < 0)
+				return ret;
+			if (ret == 0) {
+				fprintf(stderr, "No such section!\n");
+				return 1;
+			}
+			return 0;
+		}
 		else
 			break;
 		argc--;
diff --git a/config.c b/config.c
index 0ff413b..49df7bd 100644
--- a/config.c
+++ b/config.c
@@ -854,6 +859,33 @@ write_err_out:
 
 }
 
+static int section_name_match (const char *buf, const char *name)
+{
+	int i = 0, j = 0, dot = 0;
+	for (; buf[i] && buf[i] != ']'; i++) {
+		if (!dot && isspace(buf[i])) {
+			dot = 1;
+			if (name[j++] != '.')
+				break;
+			for (i++; isspace(buf[i]); i++)
+				; /* do nothing */
+			if (buf[i] != '"')
+				break;
+			continue;
+		}
+		if (buf[i] == '\\' && dot)
+			i++;
+		else if (buf[i] == '"' && dot) {
+			for (i++; isspace(buf[i]); i++)
+				; /* do_nothing */
+			break;
+		}
+		if (buf[i] != name[j++])
+			break;
+	}
+	return (buf[i] == ']' && name[j] == 0);
+}
+
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
 	int ret = 0;
@@ -885,40 +917,15 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 		int length;
 		for (i = 0; buf[i] && isspace(buf[i]); i++)
 			; /* do nothing */
-		if (buf[i] == '[') {
+		if (buf[i] == '[' && section_name_match (&buf[i+1], old_name)) {
 			/* it's a section */
-			int j = 0, dot = 0;
-			for (i++; buf[i] && buf[i] != ']'; i++) {
-				if (!dot && isspace(buf[i])) {
-					dot = 1;
-					if (old_name[j++] != '.')
-						break;
-					for (i++; isspace(buf[i]); i++)
-						; /* do nothing */
-					if (buf[i] != '"')
-						break;
-					continue;
-				}
-				if (buf[i] == '\\' && dot)
-					i++;
-				else if (buf[i] == '"' && dot) {
-					for (i++; isspace(buf[i]); i++)
-						; /* do_nothing */
-					break;
-				}
-				if (buf[i] != old_name[j++])
-					break;
-			}
-			if (buf[i] == ']' && old_name[j] == 0) {
-				/* old_name matches */
-				ret++;
-				store.baselen = strlen(new_name);
-				if (!store_write_section(out_fd, new_name)) {
-					ret = write_error();
-					goto out;
-				}
-				continue;
+			ret++;
+			store.baselen = strlen(new_name);
+			if (!store_write_section(out_fd, new_name)) {
+				ret = write_error();
+				goto out;
 			}
+			continue;
 		}
 		length = strlen(buf);
 		if (write_in_full(out_fd, buf, length) != length) {
@@ -934,3 +941,58 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 	return ret;
 }
 
+int git_config_remove_section(const char *name)
+{
+	int ret = 0;
+	char *config_filename;
+	struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
+	int out_fd;
+	int removing = 0;
+	char buf[1024];
+
+	config_filename = getenv(CONFIG_ENVIRONMENT);
+	if (!config_filename) {
+		config_filename = getenv(CONFIG_LOCAL_ENVIRONMENT);
+		if (!config_filename)
+			config_filename  = git_path("config");
+	}
+	config_filename = xstrdup(config_filename);
+	out_fd = hold_lock_file_for_update(lock, config_filename, 0);
+	if (out_fd < 0) {
+		ret = error("Could not lock config file!");
+		goto out;
+	}
+
+	if (!(config_file = fopen(config_filename, "rb"))) {
+		ret = error("Could not open config file!");
+		goto out;
+	}
+
+	while (fgets(buf, sizeof(buf), config_file)) {
+		int i;
+		int length;
+		for (i = 0; buf[i] && isspace(buf[i]); i++)
+			; /* do nothing */
+		if (buf[i] == '[') {
+			if (section_name_match (&buf[i + 1], name)) {
+				/* name matches */
+				ret++;
+				removing = 1;
+			} else
+				removing = 0;
+		}
+		if (removing)
+			continue;
+		length = strlen(buf);
+		if (write_in_full(out_fd, buf, length) != length) {
+			ret = write_error();
+			goto out;
+		}
+	}
+	fclose(config_file);
+	if (close(out_fd) || commit_lock_file(lock) < 0)
+			ret = error("Cannot commit config file!");
+ out:
+	free(config_filename);
+	return ret;
+}
diff --git a/cache.h b/cache.h
index 8bbc142..585a9b4 100644
--- a/cache.h
+++ b/cache.h
@@ -438,6 +439,7 @@ extern int git_config_bool(const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
+extern int git_config_remove_section(const char *);
 extern int check_repository_format_version(const char *var, const char *value);
 
 #define MAX_GITNAME (1000)

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Alex Riesen]

On 3/1/07, Paolo Bonzini <bonzini@gnu.org> wrote:
> As per the subject.  The existing --rename-section option is
> undocumented, while --remove-section is added by this patch based on the
> code I had written for the "git branch" patch.

You desperately need tests for this code.

[PATCH] add test for git-config --remove-section

[Johannes Schindelin]

This adds a simple test if a section is properly removed, even if the 
section header is repeated in the config.

Not-yet-Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Waits-for-ack-by: Paolo Bonzini <bonzini@gnu.org>

---
	On Thu, 1 Mar 2007, Alex Riesen wrote:

	> On 3/1/07, Paolo Bonzini <bonzini@gnu.org> wrote:
	> > As per the subject.  The existing --rename-section option is
	> > undocumented, while --remove-section is added by this patch 
	> > based on the code I had written for the "git branch" patch.
	> 
	> You desperately need tests for this code.

	Something like this? Totally untested (of course)...

 t/t1300-repo-config.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)


diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3753e9f..6b15a3e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -391,6 +391,22 @@ EOF
 
 test_expect_success "rename succeeded" "diff -u expect .git/config"
 
+test_expect_success "remove section" "git config --remove-section branch.zwei"
+
+cat > expect << EOF
+# Hallo
+	#Bello
+[branch "zwei"]
+	x = 1
+[branch "zwei"]
+	y = 1
+[branch "drei"]
+weird
+EOF
+
+test_expect_success "section was removed properly" \
+	"diff -u expect .git/config"
+
 test_expect_success numbers '
 
 	git-config kilo.gram 1k &&

Re: [PATCH] add test for git-config --remove-section

[Paolo Bonzini]

Johannes Schindelin wrote:
> This adds a simple test if a section is properly removed, even if the 
> section header is repeated in the config.
> 
> Not-yet-Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Waits-for-ack-by: Paolo Bonzini <bonzini@gnu.org>

Thanks!

Paolo

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Johannes Schindelin]

Hi,

On Thu, 1 Mar 2007, Paolo Bonzini wrote:

> As per the subject.  The existing --rename-section option is 
> undocumented, while --remove-section is added by this patch based on the 
> code I had written for the "git branch" patch.

Am I right assuming that you copied rename_section(), and modified it to 
be remove_section()?

How about modifying rename_section() so that if new_name == NULL, it 
removes the section?

But I like the way you extracted section_name_match(). Makes the code much 
more readable.

Ciao,
Dscho

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Paolo Bonzini]

> Am I right assuming that you copied rename_section(), and modified it to 
> be remove_section()?

Yes, after extracting section_name_match.

> How about modifying rename_section() so that if new_name == NULL, it 
> removes the section?

It's not so immediate, because rename_section has to print all the non-section lines anyway, while remove_section has to remove them too.  I agree though that there is some duplicate code in getting the filename to open.

Thanks for writing the test, having an example will make my future work easier.

Paolo

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Johannes Schindelin]

Hi,

On Fri, 2 Mar 2007, Paolo Bonzini wrote:

> > How about modifying rename_section() so that if new_name == NULL, it 
> > removes the section?
> 
> It's not so immediate, because rename_section has to print all the 
> non-section lines anyway, while remove_section has to remove them too.

Of course you would have to move "int removing" and the related two lines 
to rename_section(), too.

The diff would be shorter, and if you then rename the function to 
rename_or_remove_section() it becomes all clearer.

Ciao,
Dscho

> Thanks for writing the test, having an example will make my future work 
> easier.

You're welcome. Knowing how useful tests are is one of the good lessons of 
Extreme Programming...

Ciao,
Dscho

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Paolo Bonzini]

> Of course you would have to move "int removing" and the related two lines 
> to rename_section(), too.
> 
> The diff would be shorter, and if you then rename the function to 
> rename_or_remove_section() it becomes all clearer.

It's more different than it looks like.  For example, remove_section needs this,

        if (buf[i] == '[') {
                if (section_name_match (&buf[i + 1], name)) {
		} else
			...
	}

while rename_section can use

        if (buf[i] == '[' && section_name_match (&buf[i+1], old_name)) {

and has more indenting in it.  The amount of indentation in the combined function is not makign things easier to read.

Paolo

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Johannes Schindelin]

Hi,

On Fri, 2 Mar 2007, Paolo Bonzini wrote:

> > Of course you would have to move "int removing" and the related two 
> > lines to rename_section(), too.
> > 
> > The diff would be shorter, and if you then rename the function to 
> > rename_or_remove_section() it becomes all clearer.
> 
> It's more different than it looks like.  For example, remove_section needs this,
> 
>         if (buf[i] == '[') {
>                 if (section_name_match (&buf[i + 1], name)) {
> 		} else
> 			...
> 	}
> 
> while rename_section can use
> 
>         if (buf[i] == '[' && section_name_match (&buf[i+1], old_name)) {

But it does not have to.

	if (buf[i] == '[') {
		if (section_name_match(&buf[i + 1], old_name) {
			if (new_name == NULL)
				remove = 1;
			else
				/* write new section */
			continue;
		}
		remove = 0;
	}
	if (!remove)
		/* write buf */

Ciao,
Dscho

Re: [PATCH] Add git-config --remove-section, document --rename-section

[Paolo Bonzini]

> 	if (buf[i] == '[') {
> 		if (section_name_match(&buf[i + 1], old_name) {
> 			if (new_name == NULL)
> 				remove = 1;
> 			else
> 				/* write new section */
> 			continue;
> 		}
> 		remove = 0;
> 	}
> 	if (!remove)
> 		/* write buf */

Try doing this and you'll see that the 8-character indentation makes it pretty hard. :-)

Paolo

[PATCH] git-config: document --rename-section, provide --remove-section

[Johannes Schindelin]

From: Paolo Bonzini <bonzini@gnu.org>

This patch documents the previously undocumented option --rename-section
and adds a new option to zap an entire section.

Hopefully-Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	I just made the changes to git_rename_section() myself, and added
	the test case (which I made a little nastier, too), then made sure
	that everything is fine.

	Paolo, do you agree with this patch? I left you as the author, 
	because you did all the hard work.

 Documentation/git-config.txt |    8 +++++
 builtin-config.c             |   15 ++++++++++-
 config.c                     |   60 +++++++++++++++++++++++++----------------
 t/t1300-repo-config.sh       |   16 +++++++++++
 4 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 6624484..68de588 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,6 +16,8 @@ SYNOPSIS
 'git-config' [--global] [type] --get-all name [value_regex]
 'git-config' [--global] [type] --unset name [value_regex]
 'git-config' [--global] [type] --unset-all name [value_regex]
+'git-config' [--global] [type] --rename-section old_name new_name
+'git-config' [--global] [type] --remove-section name
 'git-config' [--global] -l | --list
 
 DESCRIPTION
@@ -74,6 +76,12 @@ OPTIONS
 --global::
 	Use global ~/.gitconfig file rather than the repository .git/config.
 
+--remove-section::
+	Remove the given section from the configuration file.
+
+--rename-section::
+	Rename the given section to a new name.
+
 --unset::
 	Remove the line matching the key from config file.
 
diff --git a/builtin-config.c b/builtin-config.c
index f1433a4..dfa403b 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -2,7 +2,7 @@
 #include "cache.h"
 
 static const char git_config_set_usage[] =
-"git-config [ --global ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --list";
+"git-config [ --global ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list";
 
 static char *key;
 static regex_t *key_regexp;
@@ -168,6 +168,19 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			}
 			return 0;
 		}
+		else if (!strcmp(argv[1], "--remove-section")) {
+			int ret;
+			if (argc != 3)
+				usage(git_config_set_usage);
+			ret = git_config_rename_section(argv[2], NULL);
+			if (ret < 0)
+				return ret;
+			if (ret == 0) {
+				fprintf(stderr, "No such section!\n");
+				return 1;
+			}
+			return 0;
+		}
 		else
 			break;
 		argc--;
diff --git a/config.c b/config.c
index 0ff413b..5611d7a 100644
--- a/config.c
+++ b/config.c
@@ -854,9 +854,37 @@ write_err_out:
 
 }
 
+static int section_name_match (const char *buf, const char *name)
+{
+	int i = 0, j = 0, dot = 0;
+	for (; buf[i] && buf[i] != ']'; i++) {
+		if (!dot && isspace(buf[i])) {
+			dot = 1;
+			if (name[j++] != '.')
+				break;
+			for (i++; isspace(buf[i]); i++)
+				; /* do nothing */
+			if (buf[i] != '"')
+				break;
+			continue;
+		}
+		if (buf[i] == '\\' && dot)
+			i++;
+		else if (buf[i] == '"' && dot) {
+			for (i++; isspace(buf[i]); i++)
+				; /* do_nothing */
+			break;
+		}
+		if (buf[i] != name[j++])
+			break;
+	}
+	return (buf[i] == ']' && name[j] == 0);
+}
+
+/* if new_name == NULL, the section is removed instead */
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
-	int ret = 0;
+	int ret = 0, remove = 0;
 	char *config_filename;
 	struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
 	int out_fd;
@@ -887,31 +915,12 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 			; /* do nothing */
 		if (buf[i] == '[') {
 			/* it's a section */
-			int j = 0, dot = 0;
-			for (i++; buf[i] && buf[i] != ']'; i++) {
-				if (!dot && isspace(buf[i])) {
-					dot = 1;
-					if (old_name[j++] != '.')
-						break;
-					for (i++; isspace(buf[i]); i++)
-						; /* do nothing */
-					if (buf[i] != '"')
-						break;
+			if (section_name_match (&buf[i+1], old_name)) {
+				ret++;
+				if (new_name == NULL) {
+					remove = 1;
 					continue;
 				}
-				if (buf[i] == '\\' && dot)
-					i++;
-				else if (buf[i] == '"' && dot) {
-					for (i++; isspace(buf[i]); i++)
-						; /* do_nothing */
-					break;
-				}
-				if (buf[i] != old_name[j++])
-					break;
-			}
-			if (buf[i] == ']' && old_name[j] == 0) {
-				/* old_name matches */
-				ret++;
 				store.baselen = strlen(new_name);
 				if (!store_write_section(out_fd, new_name)) {
 					ret = write_error();
@@ -919,7 +928,10 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 				}
 				continue;
 			}
+			remove = 0;
 		}
+		if (remove)
+			continue;
 		length = strlen(buf);
 		if (write_in_full(out_fd, buf, length) != length) {
 			ret = write_error();
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3753e9f..655d1e6 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -391,6 +391,22 @@ EOF
 
 test_expect_success "rename succeeded" "git diff expect .git/config"
 
+cat >> .git/config << EOF
+  [branch "zwei"] a = 1 [branch "vier"]
+EOF
+
+test_expect_success "remove section" "git config --remove-section branch.zwei"
+
+cat > expect << EOF
+# Hallo
+	#Bello
+[branch "drei"]
+weird
+EOF
+
+test_expect_success "section was removed properly" \
+	"diff -u expect .git/config"
+
 test_expect_success numbers '
 
 	git-config kilo.gram 1k &&
-- 
1.5.0.2.2488.gdffb-dirty

Re: [PATCH] git-config: document --rename-section, provide --remove-section

[Paolo Bonzini]

> 	Paolo, do you agree with this patch?

Sure I do.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>