Re: [PATCH RFC] Add a config verbose option fetch and push

Re: [PATCH RFC] Add a config verbose option fetch and push

[Nathan W. Panike]

---
>> 
>> +fetch.verbose::
>> +	If true, it is the same as setting "-v" on the command line. If it is
>> +	false or not defined, git will use the command line parameters to
>> +	decide the verboseness of fetch.
>> +
> 
> Don't you usually use the configured option as the default, and 
> then let the command line options override it (e.g., by specifying
> --no-verbose).
> 
> //Peter
> 

This patch fixes this objection.

 Documentation/config.txt |    7 +++++++
 builtin/fetch.c          |    7 +++++++
 builtin/push.c           |    7 +++++++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 39140ba..fc88d02 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -860,6 +860,9 @@ fetch.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
+fetch.verbose::
+	If true, it is the same as setting "-v" on the command line.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
@@ -1495,6 +1498,10 @@ push.default::
 * `tracking` push the current branch to its upstream branch.
 * `current` push the current branch to a branch of the same name.
 
+push.verbose::
+	If true, it is the same as using the '-v' flag on the command
+	line.
+
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
 	rebase. False by default.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8470850..f4832fe 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -885,6 +885,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	return exit_code;
 }
 
+static int git_fetch_verbose_config(const char *var,const char *value, void *dummy)
+{
+	if(!strcmp("fetch.verbose",var))
+		verbosity = git_config_maybe_bool(NULL,value);
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -897,6 +903,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++)
 		strbuf_addf(&default_rla, " %s", argv[i]);
 
+	git_config(git_fetch_verbose_config,NULL);
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
 
diff --git a/builtin/push.c b/builtin/push.c
index f4358b9..e907b11 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -202,6 +202,12 @@ static int do_push(const char *repo, int flags)
 	return !!errs;
 }
 
+static int git_push_verbose_config(const char *var, const char *value, void *d)
+{
+	if(!strcmp("push.verbose",var))
+		verbosity = git_config_maybe_bool(NULL,value);
+}
+
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
@@ -229,6 +235,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	};
 
 	git_config(git_default_config, NULL);
+	git_config(git_push_verbose_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
-- 
1.7.1.97.gf85c7

Re: [PATCH RFC] Add a config verbose option fetch and push

[Ævar Arnfjörð Bjarmason]

Since Peter Kjellerstedt wanted --ff-only, and you want --verbose. I
wonder whether a better solution wouldn't be to farm this
functionality out to the config parser.

I.e. you'd do something like:

    static struct option builtin_fetch_options[] = {
        OPT__PROGRAM_NAME("fetch"), /* this is new */
	    OPT__VERBOSITY(&verbosity),
    	OPT_BOOLEAN(0, "all", &all,
	    	    "fetch from all remotes"),
        ...

And then in your .gitconfig:

    [fetch "option"]
        verbose = 1

Is there any reason not to add such a general facility?

Re: [PATCH RFC] Add a config verbose option fetch and push

[Thomas Rast]

Ævar Arnfjörð Bjarmason wrote:
> Since Peter Kjellerstedt wanted --ff-only, and you want --verbose. I
> wonder whether a better solution wouldn't be to farm this
> functionality out to the config parser.
[...]
>     [fetch "option"]
>         verbose = 1
> 
> Is there any reason not to add such a general facility?

That would completely ruin the scriptability of almost all commands.
Imagine the user added the following options as default:

  add --edit
  checkout --patch
  cherry-pick --no-commit
  commit --amend
  pull --rebase

I'm sure you can find one option that changes the command in something
completely different *for every command*.

In fact even pull --ff-only has the same problem since it will refuse
the merge in cases where an unmodified pull would go through.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH RFC] Add a config verbose option fetch and push

[Ævar Arnfjörð Bjarmason]

On Sat, May 22, 2010 at 10:44, Thomas Rast <trast@student.ethz.ch> wrote:
> Ævar Arnfjörð Bjarmason wrote:
> That would completely ruin the scriptability of almost all commands.
> Imagine the user added the following options as default:
>
>  add --edit
>  checkout --patch
>  cherry-pick --no-commit
>  commit --amend
>  pull --rebase
>
> I'm sure you can find one option that changes the command in something
> completely different *for every command*.

Sure. But so would adding this as git-add to your $PATH:

    #!/bin/sh
    /usr/lib/git-core/git-add --edit $@

Git already has plenty of ways to shoot yourself in the foot. I don't
see how it's worse if that's done through some generalized facility
which results in less special-case code in individual tools.

Re: [PATCH RFC] Add a config verbose option fetch and push

[Thomas Rast]

Ævar Arnfjörð Bjarmason wrote:
> On Sat, May 22, 2010 at 10:44, Thomas Rast <trast@student.ethz.ch> wrote:
> > Ævar Arnfjörð Bjarmason wrote:
> > That would completely ruin the scriptability of almost all commands.
> > Imagine the user added the following options as default:
> >  add --edit
[...]
> > I'm sure you can find one option that changes the command in something
> > completely different *for every command*.
> 
> Sure. But so would adding this as git-add to your $PATH:
> 
>     #!/bin/sh
>     /usr/lib/git-core/git-add --edit $@

Two points:

* This way is not documented in git-config(1), as the proposed
  interface would have to be; hence, it is not "official".

* More importantly, it doesn't work; for builtins such as git-add, not
  even if you put it under the `git --exec-path` (yes, I've tested
  this).

> Git already has plenty of ways to shoot yourself in the foot.

Can't argue with that.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH RFC] Add a config verbose option fetch and push

[Ævar Arnfjörð Bjarmason]

On Sat, May 22, 2010 at 13:34, Thomas Rast <trast@student.ethz.ch> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>> On Sat, May 22, 2010 at 10:44, Thomas Rast <trast@student.ethz.ch> wrote:
>> > Ævar Arnfjörð Bjarmason wrote:
>> > That would completely ruin the scriptability of almost all commands.
>> > Imagine the user added the following options as default:
>> >  add --edit
> [...]
>> > I'm sure you can find one option that changes the command in something
>> > completely different *for every command*.
>>
>> Sure. But so would adding this as git-add to your $PATH:
>>
>>     #!/bin/sh
>>     /usr/lib/git-core/git-add --edit $@
>
> Two points:
>
> * This way is not documented in git-config(1), as the proposed
>  interface would have to be; hence, it is not "official".
>
> * More importantly, it doesn't work; for builtins such as git-add, not
>  even if you put it under the `git --exec-path` (yes, I've tested
>  this).

I actually tested it too and found that it didn't work. Then thought
"meh, it's pseudocode" and pressed "Send".

Aside from the specific implementation it's easy to make that work the
right way. You can alias the git command itself and munge its
arguments before passing them on to Git itself.

Anyway, this feature isn't something I actually care about. I only
wanted to suggest that if we're going to get lots of proposals to add
a specific config flag for some specific option in some specific
tool. That maybe it would be easier for everyone if there was some
general facility to do so. It would cut down on special-case code in
individual tools.

>> Git already has plenty of ways to shoot yourself in the foot.
>
> Can't argue with that.

And perhaps a general facility might actually improve scriptability. A
determined user is going to override Git anyway, even if that means
some gross hack involving munging $PATH and overriding of Git
itself. At least if such users were pointed to a general facility
script writers could set GIT_IGNORE_CRAZY_USER_DIRECTIVES=1 or
something like that.

[PATCH RFC] Add a config verbose option fetch and push

[Nathan W. Panike]

---
There are a couple projects that I follow where I find it useful to always
set the verbose flag.  Setting this in the configuration file would let me 
avoid the times I forget to set it on the command line. I decided to do both
push and fetch at the same time.

 Documentation/config.txt |   10 ++++++++++
 builtin/fetch.c          |    7 +++++++
 builtin/push.c           |    7 +++++++
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 39140ba..8e8a760 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -860,6 +860,11 @@ fetch.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
+fetch.verbose::
+	If true, it is the same as setting "-v" on the command line. If it is
+	false or not defined, git will use the command line parameters to
+	decide the verboseness of fetch.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
@@ -1495,6 +1500,11 @@ push.default::
 * `tracking` push the current branch to its upstream branch.
 * `current` push the current branch to a branch of the same name.
 
+push.verbose::
+	If true, it is the same as using the '-v' flag on the command
+	line. If it is false, git will use the command line to determine
+	the verboseness of a push.
+
 rebase.stat::
 	Whether to show a diffstat of what changed upstream since the last
 	rebase. False by default.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8470850..b2891b1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -885,6 +885,12 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 	return exit_code;
 }
 
+static int git_fetch_verbose_config(const char *var,const char *value, void *dummy)
+{
+	if(!strcmp("fetch.verbose",var) && !verbosity)
+		verbosity = git_config_maybe_bool(NULL,value);
+}
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -899,6 +905,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
+	git_config(git_fetch_verbose_config,NULL);
 
 	if (all) {
 		if (argc == 1)
diff --git a/builtin/push.c b/builtin/push.c
index f4358b9..6779031 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -202,6 +202,12 @@ static int do_push(const char *repo, int flags)
 	return !!errs;
 }
 
+static int git_push_verbose_config(const char *var, const char *value, void *d)
+{
+	if(!strcmp("push.verbose",var) && !verbosity)
+		verbosity = git_config_maybe_bool(NULL,value);
+}
+
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
@@ -230,6 +236,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+	git_config(git_push_verbose_config,NULL);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
 		die("--delete is incompatible with --all, --mirror and --tags");
-- 
1.7.1

RE: [PATCH RFC] Add a config verbose option fetch and push

[Peter Kjellerstedt]

> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Nathan W. Panike
> Sent: den 21 maj 2010 15:26
> To: git@vger.kernel.org
> Subject: [PATCH RFC] Add a config verbose option fetch and push
> 
> ---
> There are a couple projects that I follow where I find it useful to always
> set the verbose flag.  Setting this in the configuration file would let me
> avoid the times I forget to set it on the command line. I decided to do both
> push and fetch at the same time.
> 
>  Documentation/config.txt |   10 ++++++++++
>  builtin/fetch.c          |    7 +++++++
>  builtin/push.c           |    7 +++++++
>  3 files changed, 24 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 39140ba..8e8a760 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -860,6 +860,11 @@ fetch.unpackLimit::
>  	especially on slow filesystems.  If not set, the value of
>  	`transfer.unpackLimit` is used instead.
> 
> +fetch.verbose::
> +	If true, it is the same as setting "-v" on the command line. If it is
> +	false or not defined, git will use the command line parameters to
> +	decide the verboseness of fetch.
> +

Don't you usually use the configured option as the default, and 
then let the command line options override it (e.g., by specifying
--no-verbose).

//Peter