git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] builtin-status: submodule summary support
@ 2008-04-12 15:05 Ping Yin
  2008-04-12 15:05 ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Ping Yin
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:05 UTC (permalink / raw)
  To: gitster; +Cc: git

The 2nd patch of v2 are splitted to 2 patches in v3 (the 1st and 3rd one)
to support bool/int behaviour of status.submodulesummary.

The other 2 patches are the same as v2.

Ping Yin (4):
      config.c: Add git_config_bool_or_int to handle bool/int variable
      git-submodule summary: --for-status option
      builtin-status: submodule summary support
      buitin-status: Add tests for submodule summary

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-12 15:05 [PATCH v3 0/4] builtin-status: submodule summary support Ping Yin
@ 2008-04-12 15:05 ` Ping Yin
  2008-04-12 15:05   ` [PATCH v3 2/4] git-submodule summary: --for-status option Ping Yin
  2008-04-13  6:26   ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Junio C Hamano
  0 siblings, 2 replies; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:05 UTC (permalink / raw)
  To: gitster; +Cc: git, Ping Yin

With git_config_bool_or_int, the caller can differentiate boolean true
and integer 1 etc.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 config.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/config.c b/config.c
index 0624494..e614456 100644
--- a/config.c
+++ b/config.c
@@ -316,6 +316,21 @@ int git_config_bool(const char *name, const char *value)
 	return git_config_int(name, value) != 0;
 }
 
+int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
+{
+	*is_bool = 1;
+	if (!value)
+		return 1;
+	if (!*value)
+		return 0;
+	if (!strcasecmp(value, "true") || !strcasecmp(value, "yes"))
+		return 1;
+	if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
+		return 0;
+	*is_bool = 0;
+	return git_config_int(name, value);
+}
+
 int git_config_string(const char **dest, const char *var, const char *value)
 {
 	if (!value)
-- 
1.5.5.23.g2a5f

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/4] git-submodule summary: --for-status option
  2008-04-12 15:05 ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Ping Yin
@ 2008-04-12 15:05   ` Ping Yin
  2008-04-12 15:05     ` [PATCH v3 3/4] builtin-status: submodule summary support Ping Yin
  2008-04-13  6:26   ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:05 UTC (permalink / raw)
  To: gitster; +Cc: git, Ping Yin

The --for-status option is mainly used by builtin-status/commit.
It adds 'Modified submodules:' line at top and  '# ' prefix to all
following lines.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-submodule.sh             |   17 ++++++++++++++++-
 t/t7401-submodule-summary.sh |   13 +++++++++++++
 2 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 56ec353..d7937a5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -342,6 +342,7 @@ set_name_rev () {
 #
 cmd_summary() {
 	summary_limit=-1
+	for_status=
 
 	# parse $args after "submodule ... summary".
 	while test $# -ne 0
@@ -350,6 +351,9 @@ cmd_summary() {
 		--cached)
 			cached="$1"
 			;;
+		--for-status)
+			for_status="$1"
+			;;
 		-n|--summary-limit)
 			if summary_limit=$(($2 + 0)) 2>/dev/null && test "$summary_limit" = "$2"
 			then
@@ -398,6 +402,12 @@ cmd_summary() {
 	)
 
 	test -n "$modules" &&
+	if test -n "$for_status"; then
+		echo "# Modified submodules:"
+		echo "#"
+	else
+		true
+	fi &&
 	git diff-index $cached --raw $head -- $modules |
 	grep -e '^:160000' -e '^:[0-7]* 160000' |
 	cut -c2- |
@@ -499,7 +509,12 @@ cmd_summary() {
 			echo
 		fi
 		echo
-	done
+	done |
+	if test -n "$for_status"; then
+		sed -e "s|^|# |" -e 's|^# $|#|'
+	else
+		cat
+	fi
 }
 #
 # List all submodules, prefixed with:
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 0f3c42a..1dbb39d 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -192,4 +192,17 @@ test_expect_success 'given commit' "
 EOF
 "
 
+test_expect_success '--for-status' "
+    git submodule summary --for-status HEAD^ >actual &&
+    diff actual - <<-EOF
+# Modified submodules:
+#
+# * sm1 $head6...0000000:
+#
+# * sm2 0000000...$head7 (2):
+#   > Add foo9
+#
+EOF
+"
+
 test_done
-- 
1.5.5.23.g2a5f

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/4] builtin-status: submodule summary support
  2008-04-12 15:05   ` [PATCH v3 2/4] git-submodule summary: --for-status option Ping Yin
@ 2008-04-12 15:05     ` Ping Yin
  2008-04-12 15:05       ` [PATCH v3 4/4] buitin-status: Add tests for submodule summary Ping Yin
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:05 UTC (permalink / raw)
  To: gitster; +Cc: git, Ping Yin

This commit teaches 'git commit/status' show 'Modified submodules'
section given by

git submodule summary --cached --for-status --summary-limit <limit>

just before 'Untracked files' section.

The <limit> is given by the config variable status.submodulesummary
to limit the submodule summary size. status.submodulesummary is a
bool/int variable with value

  - false or 0 by default to disable the summary or
  - positive number to limit the summary size or
  - true or negative number to unlimit the summary size

Also mention status.submodulesummary in the documentation.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 Documentation/git-status.txt |    5 +++++
 wt-status.c                  |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 3ea269a..ea4376a 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -52,6 +52,11 @@ If the config variable `status.relativePaths` is set to false, then all
 paths shown are relative to the repository root, not to the current
 directory.
 
+If `status.submodulesummary` is set to a non zero number or true (identical
+to -1 or an unlimited number), the submodule summary will be enabled and a
+summary of commits for modified submodules will be shown (see --summary-limit
+option of linkgit:git-submodule[1]).
+
 See Also
 --------
 linkgit:gitignore[5]
diff --git a/wt-status.c b/wt-status.c
index b3fd57b..3baa128 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -8,9 +8,11 @@
 #include "revision.h"
 #include "diffcore.h"
 #include "quote.h"
+#include "run-command.h"
 
 int wt_status_relative_paths = 1;
 int wt_status_use_color = -1;
+int wt_status_submodule_summary;
 static char wt_status_colors[][COLOR_MAXLEN] = {
 	"",         /* WT_STATUS_HEADER: normal */
 	"\033[32m", /* WT_STATUS_UPDATED: green */
@@ -219,6 +221,38 @@ static void wt_status_print_changed(struct wt_status *s)
 	rev.diffopt.format_callback_data = s;
 	run_diff_files(&rev, 0);
 }
+static void wt_status_print_submodule_summary(struct wt_status *s)
+{
+	struct child_process sm_summary;
+	char summary_limit[64];
+	char index[PATH_MAX];
+	const char *env[] = { index, NULL };
+	const char *argv[] = {
+		"submodule",
+		"summary",
+		"--cached",
+		"--for-status",
+		"--summary-limit",
+		summary_limit,
+		s->amend ? "HEAD^" : "HEAD",
+		NULL
+	};
+
+	if (!wt_status_submodule_summary)
+		return;
+
+	sprintf(summary_limit, "%d", wt_status_submodule_summary);
+	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
+
+	memset(&sm_summary, 0, sizeof(sm_summary));
+	sm_summary.argv = argv;
+	sm_summary.env = env;
+	sm_summary.git_cmd = 1;
+	sm_summary.no_stdin = 1;
+	fflush(s->fp);
+	sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
+	run_command(&sm_summary);
+}
 
 static void wt_status_print_untracked(struct wt_status *s)
 {
@@ -308,6 +342,7 @@ void wt_status_print(struct wt_status *s)
 	}
 
 	wt_status_print_changed(s);
+	wt_status_print_submodule_summary(s);
 	wt_status_print_untracked(s);
 
 	if (s->verbose && !s->is_initial)
@@ -330,6 +365,13 @@ void wt_status_print(struct wt_status *s)
 
 int git_status_config(const char *k, const char *v)
 {
+	if (!strcmp(k, "status.submodulesummary")) {
+		int is_bool;
+		wt_status_submodule_summary = git_config_bool_or_int(k, v, &is_bool);
+		if (is_bool && wt_status_submodule_summary)
+			wt_status_submodule_summary = -1;
+		return 0;
+	}
 	if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
 		wt_status_use_color = git_config_colorbool(k, v, -1);
 		return 0;
-- 
1.5.5.23.g2a5f

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] buitin-status: Add tests for submodule summary
  2008-04-12 15:05     ` [PATCH v3 3/4] builtin-status: submodule summary support Ping Yin
@ 2008-04-12 15:05       ` Ping Yin
  2008-04-12 15:13         ` Ping Yin
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:05 UTC (permalink / raw)
  To: gitster; +Cc: git, Ping Yin

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 t/t7502-status.sh |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 134 insertions(+), 0 deletions(-)

diff --git a/t/t7502-status.sh b/t/t7502-status.sh
index cd08516..33882c9 100755
--- a/t/t7502-status.sh
+++ b/t/t7502-status.sh
@@ -149,4 +149,138 @@ test_expect_success 'status of partial commit excluding new file in index' '
 	test_cmp expect output
 '
 
+test_expect_success "setup status submodule summary" '
+	test_create_repo sm &&
+	cd sm &&
+	: >foo &&
+	git add foo &&
+	git commit -m "Add foo" &&
+	cd .. &&
+	git add sm
+'
+
+cat > expect <<EOF
+# On branch master
+# Changes to be committed:
+#   (use "git reset HEAD <file>..." to unstage)
+#
+#	new file:   dir2/added
+#	new file:   sm
+#
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   dir1/modified
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+EOF
+test_expect_success "status submodule summary is disabled by default" '
+	git status > output &&
+	git diff expect output
+'
+
+head=$(cd sm && git rev-parse --short=7 --verify HEAD)
+
+cat > expect <<EOF
+# On branch master
+# Changes to be committed:
+#   (use "git reset HEAD <file>..." to unstage)
+#
+#	new file:   dir2/added
+#	new file:   sm
+#
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   dir1/modified
+#
+# Modified submodules:
+#
+# * sm 0000000...$head (1):
+#   > Add foo
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+EOF
+test_expect_success "status submodule summary" '
+	git config status.submodulesummary 10 &&
+	git status > output &&
+	git diff expect output
+'
+
+
+cat > expect <<EOF
+# On branch master
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   dir1/modified
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+no changes added to commit (use "git add" and/or "git commit -a")
+EOF
+test_expect_success "status submodule summary (clean submodule)" '
+	git commit -m "commit submodule" &&
+	git config status.submodulesummary 10 &&
+	! git status > output &&
+	git diff expect output
+'
+
+cat > expect <<EOF
+# On branch master
+# Changes to be committed:
+#   (use "git reset HEAD^1 <file>..." to unstage)
+#
+#	new file:   dir2/added
+#	new file:   sm
+#
+# Changed but not updated:
+#   (use "git add <file>..." to update what will be committed)
+#
+#	modified:   dir1/modified
+#
+# Modified submodules:
+#
+# * sm 0000000...$head (1):
+#   > Add foo
+#
+# Untracked files:
+#   (use "git add <file>..." to include in what will be committed)
+#
+#	dir1/untracked
+#	dir2/modified
+#	dir2/untracked
+#	expect
+#	output
+#	untracked
+EOF
+test_expect_success "status submodule summary (--amend)" '
+	git config status.submodulesummary 10 &&
+	git status --amend > output &&
+	git diff expect output
+'
+
 test_done
-- 
1.5.5.23.g2a5f

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 4/4] buitin-status: Add tests for submodule summary
  2008-04-12 15:05       ` [PATCH v3 4/4] buitin-status: Add tests for submodule summary Ping Yin
@ 2008-04-12 15:13         ` Ping Yin
  0 siblings, 0 replies; 11+ messages in thread
From: Ping Yin @ 2008-04-12 15:13 UTC (permalink / raw)
  To: gitster; +Cc: git, Ping Yin

s/buitin/builtin/ in the title


-- 
Ping Yin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-12 15:05 ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Ping Yin
  2008-04-12 15:05   ` [PATCH v3 2/4] git-submodule summary: --for-status option Ping Yin
@ 2008-04-13  6:26   ` Junio C Hamano
  2008-04-13  7:28     ` Ping Yin
  2008-04-13  7:30     ` Ping Yin
  1 sibling, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2008-04-13  6:26 UTC (permalink / raw)
  To: Ping Yin; +Cc: git

Ping Yin <pkufranky@gmail.com> writes:

> With git_config_bool_or_int, the caller can differentiate boolean true
> and integer 1 etc.
>
> Signed-off-by: Ping Yin <pkufranky@gmail.com>
> ---
>  config.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/config.c b/config.c
> index 0624494..e614456 100644
> --- a/config.c
> +++ b/config.c
> @@ -316,6 +316,21 @@ int git_config_bool(const char *name, const char *value)
>  	return git_config_int(name, value) != 0;
>  }
>  
> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
> +{
> +	*is_bool = 1;
> +	if (!value)
> +		return 1;
> +	if (!*value)
> +		return 0;
> +	if (!strcasecmp(value, "true") || !strcasecmp(value, "yes"))
> +		return 1;
> +	if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
> +		return 0;
> +	*is_bool = 0;
> +	return git_config_int(name, value);
> +}
> +

I expected git_config_bool() to be implemented in terms of this new
function to avoid code duplication if we were actually going to do this.

You also need an external declaration in a header file for its users.

---
 cache.h  |    1 +
 config.c |   10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 2a1e7ec..50b28fa 100644
--- a/cache.h
+++ b/cache.h
@@ -692,6 +692,7 @@ extern int git_parse_long(const char *, long *);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_config_int(const char *, const char *);
 extern unsigned long git_config_ulong(const char *, const char *);
+extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
diff --git a/config.c b/config.c
index 0624494..5ea18ef 100644
--- a/config.c
+++ b/config.c
@@ -303,8 +303,9 @@ unsigned long git_config_ulong(const char *name, const char *value)
 	return ret;
 }
 
-int git_config_bool(const char *name, const char *value)
+int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
+	*is_bool = 1;
 	if (!value)
 		return 1;
 	if (!*value)
@@ -313,9 +314,16 @@ int git_config_bool(const char *name, const char *value)
 		return 1;
 	if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
 		return 0;
+	*is_bool = 0;
 	return git_config_int(name, value) != 0;
 }
 
+int git_config_bool(const char *name, const char *value)
+{
+	int discard;
+	return git_config_bool_or_int(name, value, &discard);
+}
+
 int git_config_string(const char **dest, const char *var, const char *value)
 {
 	if (!value)

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-13  6:26   ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Junio C Hamano
@ 2008-04-13  7:28     ` Ping Yin
  2008-04-13  7:30     ` Ping Yin
  1 sibling, 0 replies; 11+ messages in thread
From: Ping Yin @ 2008-04-13  7:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sun, Apr 13, 2008 at 2:26 PM, Junio C Hamano <junio@pobox.com> wrote:
> Ping Yin <pkufranky@gmail.com> writes:
>
>  > With git_config_bool_or_int, the caller can differentiate boolean true
>  > and integer 1 etc.
>  >
>  > Signed-off-by: Ping Yin <pkufranky@gmail.com>
>  > ---
>  >  config.c |   15 +++++++++++++++
>  >  1 files changed, 15 insertions(+), 0 deletions(-)
>  >
>  > diff --git a/config.c b/config.c
>  > index 0624494..e614456 100644
>  > --- a/config.c
>  > +++ b/config.c
>  > @@ -316,6 +316,21 @@ int git_config_bool(const char *name, const char *value)
>  >       return git_config_int(name, value) != 0;
>  >  }
>  >
>  > +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
>  > +{
>  > +     *is_bool = 1;
>  > +     if (!value)
>  > +             return 1;
>  > +     if (!*value)
>  > +             return 0;
>  > +     if (!strcasecmp(value, "true") || !strcasecmp(value, "yes"))
>  > +             return 1;
>  > +     if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
>  > +             return 0;
>  > +     *is_bool = 0;
>  > +     return git_config_int(name, value);
>  > +}
>  > +
>
>  I expected git_config_bool() to be implemented in terms of this new
>  function to avoid code duplication if we were actually going to do this.
>
>  You also need an external declaration in a header file for its users.
>
>  ---
>   cache.h  |    1 +
>   config.c |   10 +++++++++-
>   2 files changed, 10 insertions(+), 1 deletions(-)
>
>  diff --git a/cache.h b/cache.h
>  index 2a1e7ec..50b28fa 100644
>  --- a/cache.h
>  +++ b/cache.h
>  @@ -692,6 +692,7 @@ extern int git_parse_long(const char *, long *);
>   extern int git_parse_ulong(const char *, unsigned long *);
>   extern int git_config_int(const char *, const char *);
>   extern unsigned long git_config_ulong(const char *, const char *);
>  +extern int git_config_bool_or_int(const char *, const char *, int *);
>   extern int git_config_bool(const char *, const char *);
>   extern int git_config_string(const char **, const char *, const char *);
>   extern int git_config_set(const char *, const char *);
>
> diff --git a/config.c b/config.c
>  index 0624494..5ea18ef 100644
>
> --- a/config.c
>  +++ b/config.c
>  @@ -303,8 +303,9 @@ unsigned long git_config_ulong(const char *name, const char *value)
>         return ret;
>   }
>
>  -int git_config_bool(const char *name, const char *value)
>
> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
>   {
>  +       *is_bool = 1;
>         if (!value)
>                 return 1;
>         if (!*value)
>  @@ -313,9 +314,16 @@ int git_config_bool(const char *name, const char *value)
>
>                 return 1;
>         if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
>                 return 0;
>  +       *is_bool = 0;
>
>         return git_config_int(name, value) != 0;
>   }
>
>  +int git_config_bool(const char *name, const char *value)
>  +{
>  +       int discard;
>  +       return git_config_bool_or_int(name, value, &discard);

Is it better to use
+       return git_config_bool_or_int(name, value, &discard) != 0;

since git_config_bool_or_int(name, value, &discard) may return an
interger neither 0 nor 1


-- 
Ping Yin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-13  6:26   ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Junio C Hamano
  2008-04-13  7:28     ` Ping Yin
@ 2008-04-13  7:30     ` Ping Yin
  2008-04-13  9:03       ` Junio C Hamano
  1 sibling, 1 reply; 11+ messages in thread
From: Ping Yin @ 2008-04-13  7:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sun, Apr 13, 2008 at 2:26 PM, Junio C Hamano <junio@pobox.com> wrote:
> Ping Yin <pkufranky@gmail.com> writes:

>  -int git_config_bool(const char *name, const char *value)
>
> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
>   {
>  +       *is_bool = 1;
>         if (!value)
>                 return 1;
>         if (!*value)
>  @@ -313,9 +314,16 @@ int git_config_bool(const char *name, const char *value)
>
>                 return 1;
>         if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
>                 return 0;
>  +       *is_bool = 0;
>
>         return git_config_int(name, value) != 0;
>   }

Should return an interger if *is_bool==0
s/!=0//


>  +int git_config_bool(const char *name, const char *value)
>  +{
>  +       int discard;
>  +       return git_config_bool_or_int(name, value, &discard);
>
>
> +}
>  +
>   int git_config_string(const char **dest, const char *var, const char *value)
>   {
>         if (!value)
>
>



-- 
Ping Yin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-13  7:30     ` Ping Yin
@ 2008-04-13  9:03       ` Junio C Hamano
  2008-04-13 19:14         ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2008-04-13  9:03 UTC (permalink / raw)
  To: Ping Yin; +Cc: git

"Ping Yin" <pkufranky@gmail.com> writes:

>>         return git_config_int(name, value) != 0;
>>   }
>
> Should return an interger if *is_bool==0
> s/!=0//

Yeah, you are right.  I do not know where that "!= 0" came from.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
  2008-04-13  9:03       ` Junio C Hamano
@ 2008-04-13 19:14         ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2008-04-13 19:14 UTC (permalink / raw)
  To: Ping Yin; +Cc: git

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

> "Ping Yin" <pkufranky@gmail.com> writes:
>
>>>         return git_config_int(name, value) != 0;
>>>   }
>>
>> Should return an interger if *is_bool==0
>> s/!=0//
>
> Yeah, you are right.  I do not know where that "!= 0" came from.

Sorry about the breakage.

The real reason the breakage did not get caught was there was nothing that
catches the breakage.  Hence this adds tests for this new feature.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 13 Apr 2008 12:11:11 -0700
Subject: [PATCH] Fix git_config_bool_or_int

The earlier one botched the return value logic between config_bool and
config_bool_and_int.  The former should normalize between 0 and 1 while
the latter should give back full range of integer values.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-config.c       |   22 ++++++++++++++++-
 config.c               |    4 +-
 t/t1300-repo-config.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index c34bc8b..10447a7 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -3,7 +3,7 @@
 #include "color.h"
 
 static const char git_config_set_usage[] =
-"git-config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int ] [ -z | --null ] [--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 | --get-color var [default] | --get-colorbool name [stdout-is-tty]";
+"git-config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--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 | --get-color var [default] | --get-colorbool name [stdout-is-tty]";
 
 static char *key;
 static regex_t *key_regexp;
@@ -16,7 +16,7 @@ static int seen;
 static char delim = '=';
 static char key_delim = ' ';
 static char term = '\n';
-static enum { T_RAW, T_INT, T_BOOL } type = T_RAW;
+static enum { T_RAW, T_INT, T_BOOL, T_BOOL_OR_INT } type = T_RAW;
 
 static int show_all_config(const char *key_, const char *value_)
 {
@@ -53,6 +53,14 @@ static int show_config(const char* key_, const char* value_)
 		sprintf(value, "%d", git_config_int(key_, value_?value_:""));
 	else if (type == T_BOOL)
 		vptr = git_config_bool(key_, value_) ? "true" : "false";
+	else if (type == T_BOOL_OR_INT) {
+		int is_bool, v;
+		v = git_config_bool_or_int(key_, value_, &is_bool);
+		if (is_bool)
+			vptr = v ? "true" : "false";
+		else
+			sprintf(value, "%d", v);
+	}
 	else
 		vptr = value_?value_:"";
 	seen++;
@@ -157,6 +165,14 @@ char *normalize_value(const char *key, const char *value)
 		else if (type == T_BOOL)
 			sprintf(normalized, "%s",
 				git_config_bool(key, value) ? "true" : "false");
+		else if (type == T_BOOL_OR_INT) {
+			int is_bool, v;
+			v = git_config_bool_or_int(key, value, &is_bool);
+			if (!is_bool)
+				sprintf(normalized, "%d", v);
+			else
+				sprintf(normalized, "%s", v ? "true" : "false");
+		}
 	}
 
 	return normalized;
@@ -273,6 +289,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			type = T_INT;
 		else if (!strcmp(argv[1], "--bool"))
 			type = T_BOOL;
+		else if (!strcmp(argv[1], "--bool-or-int"))
+			type = T_BOOL_OR_INT;
 		else if (!strcmp(argv[1], "--list") || !strcmp(argv[1], "-l")) {
 			if (argc != 2)
 				usage(git_config_set_usage);
diff --git a/config.c b/config.c
index 5ea18ef..b0ada51 100644
--- a/config.c
+++ b/config.c
@@ -315,13 +315,13 @@ int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 	if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
 		return 0;
 	*is_bool = 0;
-	return git_config_int(name, value) != 0;
+	return git_config_int(name, value);
 }
 
 int git_config_bool(const char *name, const char *value)
 {
 	int discard;
-	return git_config_bool_or_int(name, value, &discard);
+	return !!git_config_bool_or_int(name, value, &discard);
 }
 
 int git_config_string(const char **dest, const char *var, const char *value)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index b36a901..a675cbb 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -595,6 +595,64 @@ test_expect_success 'set --int' '
 
 rm .git/config
 
+cat >expect <<\EOF
+[bool]
+	true1 = true
+	true2 = true
+	false1 = false
+	false2 = false
+[int]
+	int1 = 0
+	int2 = 1
+	int3 = -1
+EOF
+
+test_expect_success 'get --bool-or-int' '
+	(
+		echo "[bool]"
+		echo true1
+		echo true2 = true
+		echo false = false
+		echo "[int]"
+		echo int1 = 0
+		echo int2 = 1
+		echo int3 = -1
+	) >>.git/config &&
+	test $(git config --bool-or-int bool.true1) = true &&
+	test $(git config --bool-or-int bool.true2) = true &&
+	test $(git config --bool-or-int bool.false) = false &&
+	test $(git config --bool-or-int int.int1) = 0 &&
+	test $(git config --bool-or-int int.int2) = 1 &&
+	test $(git config --bool-or-int int.int3) = -1
+
+'
+
+rm .git/config
+cat >expect <<\EOF
+[bool]
+	true1 = true
+	false1 = false
+	true2 = true
+	false2 = false
+[int]
+	int1 = 0
+	int2 = 1
+	int3 = -1
+EOF
+
+test_expect_success 'set --bool-or-int' '
+	git config --bool-or-int bool.true1 true &&
+	git config --bool-or-int bool.false1 false &&
+	git config --bool-or-int bool.true2 yes &&
+	git config --bool-or-int bool.false2 no &&
+	git config --bool-or-int int.int1 0 &&
+	git config --bool-or-int int.int2 1 &&
+	git config --bool-or-int int.int3 -1 &&
+	test_cmp expect .git/config
+'
+
+rm .git/config
+
 git config quote.leading " test"
 git config quote.ending "test "
 git config quote.semicolon "test;test"
-- 
1.5.5.104.ge4331

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-04-13 19:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-12 15:05 [PATCH v3 0/4] builtin-status: submodule summary support Ping Yin
2008-04-12 15:05 ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Ping Yin
2008-04-12 15:05   ` [PATCH v3 2/4] git-submodule summary: --for-status option Ping Yin
2008-04-12 15:05     ` [PATCH v3 3/4] builtin-status: submodule summary support Ping Yin
2008-04-12 15:05       ` [PATCH v3 4/4] buitin-status: Add tests for submodule summary Ping Yin
2008-04-12 15:13         ` Ping Yin
2008-04-13  6:26   ` [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable Junio C Hamano
2008-04-13  7:28     ` Ping Yin
2008-04-13  7:30     ` Ping Yin
2008-04-13  9:03       ` Junio C Hamano
2008-04-13 19:14         ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).