Re: [PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-10 Thread garciagj 

El 2013-06-08 17:25, Ramkumar Ramachandra escribió:

Jorge Juan Garcia Garcia wrote:

Some people always run 'git status -s'.
The configuration variable status.short allows to set it by default.


Good feature.

@@ -1112,6 +1112,15 @@ static int git_status_config(const char *k, 
const char *v, void *cb)

s-submodule_summary = -1;
return 0;
}
+   if (!strcmp(k, status.short)) {
+   if (!v)
+   return config_error_nonbool(k);
+   if (git_config_bool(k,v)) {
+   status_format = STATUS_FORMAT_SHORT;
+   wt_shortstatus_print(s);
+   }
+   return 0;
+   }


Incorrect.  This is the wrong place to use config_error_nonbool():
this is very much a bool, and a [status] short in ~/.gitconfig
should not error out (all boolean variables behave in the same
manner).  When in doubt, consult config_error_nonbool(); there's
clearly a comment stating:


Ok. We will change it.



/*
 * Call this to report error for your variable that should not
 * get a boolean value (i.e. [my] var means true).
 */

Also, why are you calling wt_shortstatus_print() here, instead of
returning control to cmd_status(), which is going to do it anyway?

Yes, it's obviously a mistake.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v3 [PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-10 Thread Junio C Hamano 
Matthieu Moy matthieu@grenoble-inp.fr writes:

 y...@ensimag.imag.fr writes:

 To: y...@ensimag.imag.fr

 Common mistake, but you're not supposed to answer y when you're
 prompted for an email ;-).

Didn't we introduce safety against this in v1.7.12.1 and later?  Is
the new release taking more than 9 months to percolate, or are there
still other codepaths that allow this to happen that we need to add
further safety?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-09 Thread Junio C Hamano 
Ramkumar Ramachandra artag...@gmail.com writes:

 Jorge Juan Garcia Garcia wrote:
 Some people always run 'git status -s'.
 The configuration variable status.short allows to set it by default.

 Good feature.

Is there a corresponding command line override added to help people
who need to defeat such a configured-in default?  Otherwise it is
not a good feature yet, but just only the beginning half.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-09 Thread Matthieu Moy 
Junio C Hamano gits...@pobox.com writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 Jorge Juan Garcia Garcia wrote:
 Some people always run 'git status -s'.
 The configuration variable status.short allows to set it by default.

 Good feature.

 Is there a corresponding command line override added to help people
 who need to defeat such a configured-in default?

Yes: git status --no-short.

Perhaps the doc for status.short should explicitely mention that
--no-short takes precedence over it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-08 Thread Jorge Juan Garcia Garcia 
Some people always run 'git status -s'.
The configuration variable status.short allows to set it by default.

Signed-off-by: Jorge Juan Garcia Garcia 
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
 Documentation/config.txt |3 +++
 builtin/commit.c |9 +
 config.c |1 +
 t/t7508-status.sh|   34 ++
 4 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..80cdf75 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2066,6 +2066,9 @@ status.relativePaths::
relative to the repository root (this was the default for Git
prior to v1.5.4).
 
+status.short::
+   Set to true to enable --short by default in linkgit:git-status[1].
+
 status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 1621dfc..0f3429f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1112,6 +1112,15 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
s-submodule_summary = -1;
return 0;
}
+   if (!strcmp(k, status.short)) {
+   if (!v)
+   return config_error_nonbool(k);
+   if (git_config_bool(k,v)) {
+   status_format = STATUS_FORMAT_SHORT;
+   wt_shortstatus_print(s);
+   }
+   return 0;
+   }
if (!strcmp(k, status.color) || !strcmp(k, color.status)) {
s-use_color = git_config_colorbool(k, v);
return 0;
diff --git a/config.c b/config.c
index 7a85ebd..85ddbf2 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,7 @@
 #include exec_cmd.h
 #include strbuf.h
 #include quote.h
+#include commit.h
 
 typedef struct config_file {
struct config_file *prev;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e2ffdac..4cb2b62 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1334,5 +1334,39 @@ test_expect_failure '.git/config ignore=all suppresses 
submodule summary' '
git config --remove-section submodule.subname 
git config -f .gitmodules  --remove-section submodule.subname
 '
+test_expect_success 'setup for testing status.short' '
+status1 
+status2
+'
+
+test_expect_success 'status.short=true same as -s' '
+git -c status.short=true status status2 
+git status -s status1 
+test_cmp status1 status2
+'
+
+test_expect_success 'status.short=true different from --no-short' '
+git -c status.short=true status status2 
+git status --no-short status1 
+test_must_fail test_cmp status1 status2
+'
+
+test_expect_success 'status.short=true weaker than --no-short' '
+git -c status.short=true status --no-short status2 
+git status --no-short status1 
+test_cmp status1 status2
+'
+
+test_expect_success 'status.short=false same as --no-short' '
+git -c status.short=false status status2 
+git status --no-short status1 
+test_cmp status1 status2
+'
+
+test_expect_success 'status.short=false weaker than -s' '
+git -c status.short=false status -s status2 
+git status -s status1 
+test_cmp status1 status2
+'
 
 test_done
-- 
1.7.8

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] status: introduce status.short to enable --short by default

2013-06-08 Thread Ramkumar Ramachandra 
Jorge Juan Garcia Garcia wrote:
 Some people always run 'git status -s'.
 The configuration variable status.short allows to set it by default.

Good feature.

 @@ -1112,6 +1112,15 @@ static int git_status_config(const char *k, const char 
 *v, void *cb)
 s-submodule_summary = -1;
 return 0;
 }
 +   if (!strcmp(k, status.short)) {
 +   if (!v)
 +   return config_error_nonbool(k);
 +   if (git_config_bool(k,v)) {
 +   status_format = STATUS_FORMAT_SHORT;
 +   wt_shortstatus_print(s);
 +   }
 +   return 0;
 +   }

Incorrect.  This is the wrong place to use config_error_nonbool():
this is very much a bool, and a [status] short in ~/.gitconfig
should not error out (all boolean variables behave in the same
manner).  When in doubt, consult config_error_nonbool(); there's
clearly a comment stating:

/*
 * Call this to report error for your variable that should not
 * get a boolean value (i.e. [my] var means true).
 */

Also, why are you calling wt_shortstatus_print() here, instead of
returning control to cmd_status(), which is going to do it anyway?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html