git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-10-21 16:30 Tuncer Ayaz
  2008-10-27 10:08 ` Nanako Shiraishi
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-10-21 16:30 UTC (permalink / raw)
  To: git; +Cc: gitster

After fixing clone -q I noticed that pull -q does not do what
it's supposed to do and implemented --quiet/--verbose by
adding it to builtin-merge and fixing two places in builtin-fetch.

I have not touched/adjusted contrib/completion/git-completion.bash
but can take a look if wanted. I think it already needs one or two
adjustments caused by recent --OPTIONS changes in master.

I've tested the following invocations with the below changes applied:
$ git pull
$ git pull -q
$ git pull -v
$ git fetch -q
$ git pull -q -v -q

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 Documentation/merge-options.txt |    8 ++++++++
 builtin-fetch.c                 |   21 +++++++++++----------
 builtin-merge.c                 |   22 +++++++++++++++-------
 git-pull.sh                     |   11 ++++++++---
 4 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index ee93d3a..b067512 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,16 +22,17 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity == VERBOSE)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity > QUIET && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity > QUIET)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -622,9 +623,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity == VERBOSE)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity == QUIET)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e7910b..76e2890 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
 }
 
 static struct option builtin_merge_options[] = {
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		"do not show a diffstat at the end of the merge",
 		PARSE_OPT_NOARG, option_parse_n },
@@ -250,7 +253,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity > QUIET)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity > QUIET)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity > QUIET && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity > QUIET)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity > QUIET)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 75c3610..c982d08 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,13 +16,17 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
+strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity=-q ;;
+	-v|--verbose)
+		verbosity=-v ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +125,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test "$curr_head" != "$orig_head"
@@ -181,5 +185,6 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
+exec git-merge $verbosity $no_stat $no_commit \
+	$squash $no_ff $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head
-- 
1.6.0.2.GIT

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-21 16:30 [PATCH] Teach/Fix pull/fetch -q/-v options Tuncer Ayaz
@ 2008-10-27 10:08 ` Nanako Shiraishi
  2008-10-28  3:21   ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Nanako Shiraishi @ 2008-10-27 10:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tuncer Ayaz, git

Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:

> After fixing clone -q I noticed that pull -q does not do what
> it's supposed to do and implemented --quiet/--verbose by
> adding it to builtin-merge and fixing two places in builtin-fetch.

Junio, may I ask what the status of this patch is? Maybe the patch was lost in the noise? The commit log message is written very differently from existing commits in the history of git, and I am thinking that maybe that is why you did not like the whole patch? Or is it lack of any test script?

I like what the quiet option to git-pull does, even though I do not care too much about the verbose option myself.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-27 10:08 ` Nanako Shiraishi
@ 2008-10-28  3:21   ` Junio C Hamano
  2008-11-01 17:23     ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2008-10-28  3:21 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Tuncer Ayaz, git

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:
>
>> After fixing clone -q I noticed that pull -q does not do what
>> it's supposed to do and implemented --quiet/--verbose by
>> adding it to builtin-merge and fixing two places in builtin-fetch.
>
> Junio, may I ask what the status of this patch is? Maybe the patch was lost in the noise? The commit log message is written very differently from existing commits in the history of git, and I am thinking that maybe that is why you did not like the whole patch? Or is it lack of any test script?

Well, perhaps you've been with us long enough to get too picky like
myself, but this was genuinely lost in the noise and my scrambling to get
back to normal.  We do not typically say "I did this, I did that", but the
first paragraph in Tuncer's message is perfectly fine.  I would probably
liked the second paragraph better if it were after --- lines (it is more
about the possible enhancements in other areas, and does not belong to the
commit log message for this change), but it is not a grave enough offence
to get the patch rejected.

The patch itself looks Ok; the lack of test script additions does indeed
bother me somewhat, but it is not too big a deal.

P.S. We are having fun at GitTogether'08 in the first half of this week,
so please expect slower response than usual.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-28  3:21   ` Junio C Hamano
@ 2008-11-01 17:23     ` Tuncer Ayaz
  2008-11-07  3:26       ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-01 17:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git

On Tue, Oct 28, 2008 at 4:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:
>>
>>> After fixing clone -q I noticed that pull -q does not do what
>>> it's supposed to do and implemented --quiet/--verbose by
>>> adding it to builtin-merge and fixing two places in builtin-fetch.
>>
>> Junio, may I ask what the status of this patch is? Maybe the patch was
>> lost in the noise? The commit log message is written very differently from
>> existing commits in the history of git, and I am thinking that maybe that is
>> why you did not like the whole patch? Or is it lack of any test script?
>
> Well, perhaps you've been with us long enough to get too picky like
> myself, but this was genuinely lost in the noise and my scrambling to get
> back to normal.  We do not typically say "I did this, I did that", but the
> first paragraph in Tuncer's message is perfectly fine.  I would probably
> liked the second paragraph better if it were after --- lines (it is more
> about the possible enhancements in other areas, and does not belong to the
> commit log message for this change), but it is not a grave enough offence
> to get the patch rejected.

Should I resend the patch with a short and simple commit message
like the following?
--8<--
Implement git-pull --quiet and --verbose by adding the
options to git-pull and fixing verbosity handling in git-fetch.
-->8--

> The patch itself looks Ok; the lack of test script additions does indeed
> bother me somewhat, but it is not too big a deal.

I took a look at t/ and am not quite sure whether I should test -v/-q
by analyzing stdout output's length & content.

I think testing -q and -v makes more sense on a global and not
git-pull or git-fetch level. For that to be possible I envision building
common verbosity-based logging functions/macros.

I don't like the idea of scanning stdout for length or content as
long as we're not sure that all errors and warnings are sent
to stderr and stdout is for info and verbose only.

> P.S. We are having fun at GitTogether'08 in the first half of this week,
> so please expect slower response than usual.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-01 17:23     ` Tuncer Ayaz
@ 2008-11-07  3:26       ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-07  3:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git

On Sat, Nov 1, 2008 at 6:23 PM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> On Tue, Oct 28, 2008 at 4:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nanako Shiraishi <nanako3@lavabit.com> writes:
>>
>>> Quoting Tuncer Ayaz <tuncer.ayaz@gmail.com>:
>>>
>>>> After fixing clone -q I noticed that pull -q does not do what
>>>> it's supposed to do and implemented --quiet/--verbose by
>>>> adding it to builtin-merge and fixing two places in builtin-fetch.
>>>
>>> Junio, may I ask what the status of this patch is? Maybe the patch was
>>> lost in the noise? The commit log message is written very differently from
>>> existing commits in the history of git, and I am thinking that maybe that is
>>> why you did not like the whole patch? Or is it lack of any test script?
>>
>> Well, perhaps you've been with us long enough to get too picky like
>> myself, but this was genuinely lost in the noise and my scrambling to get
>> back to normal.  We do not typically say "I did this, I did that", but the
>> first paragraph in Tuncer's message is perfectly fine.  I would probably
>> liked the second paragraph better if it were after --- lines (it is more
>> about the possible enhancements in other areas, and does not belong to the
>> commit log message for this change), but it is not a grave enough offence
>> to get the patch rejected.
>
> Should I resend the patch with a short and simple commit message
> like the following?
> --8<--
> Implement git-pull --quiet and --verbose by adding the
> options to git-pull and fixing verbosity handling in git-fetch.
> -->8--
>
>> The patch itself looks Ok; the lack of test script additions does indeed
>> bother me somewhat, but it is not too big a deal.
>
> I took a look at t/ and am not quite sure whether I should test -v/-q
> by analyzing stdout output's length & content.
>
> I think testing -q and -v makes more sense on a global and not
> git-pull or git-fetch level. For that to be possible I envision building
> common verbosity-based logging functions/macros.
>
> I don't like the idea of scanning stdout for length or content as
> long as we're not sure that all errors and warnings are sent
> to stderr and stdout is for info and verbose only.
>
>> P.S. We are having fun at GitTogether'08 in the first half of this week,
>> so please expect slower response than usual.
>

Please see my next patch revision arriving here in a moment.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-17 11:03     ` Constantine Plotnikov
@ 2008-11-17 22:24       ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-17 22:24 UTC (permalink / raw)
  To: Constantine Plotnikov; +Cc: git

On Mon, Nov 17, 2008 at 12:03 PM, Constantine Plotnikov
<constantine.plotnikov@gmail.com> wrote:
> On Mon, Nov 17, 2008 at 1:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>>
>>> I think you need to have something like the following applied on top of
>>> what's in pu to be able to use "pull -v -v -v" and be able to count the
>>> occurrences via parse-options.c. What do you think?
>>
> I'm just interested why not just optional level argument to verbosity
> like --verbose=2 or -v2?

I'm not really sure which one is better but do prefer the
"-v -v -v" way as it seems to be common practice in tools
that support it. Also I don't want to make git cli different than
most other cli tools. This does not mean that common practice
is the best choice.

I think allowing an integer param makes our cli worse and
allows usage like --verbose=25. verbose=25 is not sane
in my personal view.

$ git pull --verbose # same as --verbose=1?
$ git pull --verbose=42 # will do the right thing always by AI?
$ git pull --verbose=0 # will do what? maybe quiet=1?

I say let's keep it simple so that no one starts getting the
idea that it's fine to have more than a handful occurrences
of -v or -q.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-17 10:51   ` Junio C Hamano
  2008-11-17 10:55     ` Tuncer Ayaz
  2008-11-17 11:03     ` Constantine Plotnikov
@ 2008-11-17 22:08     ` Tuncer Ayaz
  2 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-17 22:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Nov 17, 2008 at 11:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>> I think you need to have something like the following applied on top of
>> what's in pu to be able to use "pull -v -v -v" and be able to count the
>> occurrences via parse-options.c. What do you think?
>
> Makes sense, except that as a futureproofing we may be better off doing
> the same for -q as well.

Please see my patch with the following subject arriving here soonish:
"[PATCH] Retain multiple -q/-v occurrences in git pull"

>> --- git-pull.sh 2008-11-17 11:32:19.000000000 +0100
>> +++ git-pull.sh.b       2008-11-17 11:33:03.000000000 +0100
>> @@ -26,7 +26,7 @@
>>         -q|--quiet)
>>                 verbosity=-q ;;
>>         -v|--verbose)
>> -               verbosity=-v ;;
>> +               verbosity="$verbosity -v" ;;
>>         -n|--no-stat|--no-summary)
>>                 no_stat=-n ;;
>>         --stat|--summary)
>>
>> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
>

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-17 10:51   ` Junio C Hamano
  2008-11-17 10:55     ` Tuncer Ayaz
@ 2008-11-17 11:03     ` Constantine Plotnikov
  2008-11-17 22:24       ` Tuncer Ayaz
  2008-11-17 22:08     ` Tuncer Ayaz
  2 siblings, 1 reply; 29+ messages in thread
From: Constantine Plotnikov @ 2008-11-17 11:03 UTC (permalink / raw)
  To: git

On Mon, Nov 17, 2008 at 1:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>> I think you need to have something like the following applied on top of
>> what's in pu to be able to use "pull -v -v -v" and be able to count the
>> occurrences via parse-options.c. What do you think?
>
I'm just interested why not just optional level argument to verbosity
like --verbose=2 or -v2?

Regards,
Constantine

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-17 10:51   ` Junio C Hamano
@ 2008-11-17 10:55     ` Tuncer Ayaz
  2008-11-17 11:03     ` Constantine Plotnikov
  2008-11-17 22:08     ` Tuncer Ayaz
  2 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-17 10:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Nov 17, 2008 at 11:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>> I think you need to have something like the following applied on top of
>> what's in pu to be able to use "pull -v -v -v" and be able to count the
>> occurrences via parse-options.c. What do you think?
>
> Makes sense, except that as a futureproofing we may be better off doing
> the same for -q as well.

What's funny is that my original patch to git-pull.sh had both :).

>> --- git-pull.sh 2008-11-17 11:32:19.000000000 +0100
>> +++ git-pull.sh.b       2008-11-17 11:33:03.000000000 +0100
>> @@ -26,7 +26,7 @@
>>         -q|--quiet)
>>                 verbosity=-q ;;
>>         -v|--verbose)
>> -               verbosity=-v ;;
>> +               verbosity="$verbosity -v" ;;
>>         -n|--no-stat|--no-summary)
>>                 no_stat=-n ;;
>>         --stat|--summary)
>>
>> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
>

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-17 10:37 ` Tuncer Ayaz
@ 2008-11-17 10:51   ` Junio C Hamano
  2008-11-17 10:55     ` Tuncer Ayaz
                       ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: Junio C Hamano @ 2008-11-17 10:51 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: git, gitster

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

> I think you need to have something like the following applied on top of
> what's in pu to be able to use "pull -v -v -v" and be able to count the
> occurrences via parse-options.c. What do you think?

Makes sense, except that as a futureproofing we may be better off doing
the same for -q as well.

> --- git-pull.sh 2008-11-17 11:32:19.000000000 +0100
> +++ git-pull.sh.b       2008-11-17 11:33:03.000000000 +0100
> @@ -26,7 +26,7 @@
>         -q|--quiet)
>                 verbosity=-q ;;
>         -v|--verbose)
> -               verbosity=-v ;;
> +               verbosity="$verbosity -v" ;;
>         -n|--no-stat|--no-summary)
>                 no_stat=-n ;;
>         --stat|--summary)
>
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15 19:23 Tuncer Ayaz
@ 2008-11-17 10:37 ` Tuncer Ayaz
  2008-11-17 10:51   ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-17 10:37 UTC (permalink / raw)
  To: git; +Cc: gitster

Junio,

I think you need to have something like the following applied on top of
what's in pu to be able to use "pull -v -v -v" and be able to count the
occurrences via parse-options.c. What do you think?

--- git-pull.sh 2008-11-17 11:32:19.000000000 +0100
+++ git-pull.sh.b       2008-11-17 11:33:03.000000000 +0100
@@ -26,7 +26,7 @@
        -q|--quiet)
                verbosity=-q ;;
        -v|--verbose)
-               verbosity=-v ;;
+               verbosity="$verbosity -v" ;;
        -n|--no-stat|--no-summary)
                no_stat=-n ;;
        --stat|--summary)

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>

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

* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-11-15 19:23 Tuncer Ayaz
  2008-11-17 10:37 ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15 19:23 UTC (permalink / raw)
  To: git; +Cc: gitster

Implement git-pull --quiet and git-pull --verbose by
adding the options to git-pull and fixing verbosity
handling in git-fetch.

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 Documentation/merge-options.txt |    8 +++++
 builtin-fetch.c                 |   19 ++++++------
 builtin-merge.c                 |   21 +++++++++----
 git-pull.sh                     |   10 ++++--
 parse-options.c                 |   22 ++++++++++++++
 parse-options.h                 |    6 ++++
 t/t5521-pull-options.sh         |   60 +++++++++++++++++++++++++++++++++++++++
 7 files changed, 126 insertions(+), 20 deletions(-)
 create mode 100755 t/t5521-pull-options.sh

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index f151cfa..7568163 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,7 +22,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -30,8 +30,7 @@ static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT__VERBOSITY(&verbosity),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +191,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +208,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity > 0)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +364,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity >= 0)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -637,9 +636,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity >= 2)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity < 0)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e7910b..7c2b90c 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static int verbosity;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -171,6 +172,7 @@ static struct option builtin_merge_options[] = {
 	OPT_CALLBACK('m', "message", &merge_msg, "message",
 		"message to be used for the merge commit (if any)",
 		option_parse_message),
+	OPT__VERBOSITY(&verbosity),
 	OPT_END()
 };
 
@@ -250,7 +252,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity >= 0)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +334,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity >= 0)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity >= 0 && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +876,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity < 0)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1019,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity >= 0)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 664fe34..a9dc713 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,13 +16,17 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
+strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity=-q ;;
+	-v|--verbose)
+		verbosity="$verbosity -v" ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +125,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"
@@ -182,4 +186,4 @@ test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
 exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head
+	"$merge_name" HEAD $merge_head $verbosity
diff --git a/parse-options.c b/parse-options.c
index fd08bb4..9eb55cc 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -484,6 +484,28 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
 	return 0;
 }
 
+int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
+			   int unset)
+{
+	int *target = opt->value;
+
+	if (unset)
+		/* --no-quiet, --no-verbose */
+		*target = 0;
+	else if (opt->short_name == 'v') {
+		if (*target >= 0)
+			(*target)++;
+		else
+			*target = 1;
+	} else {
+		if (*target <= 0)
+			(*target)--;
+		else
+			*target = -1;
+	}
+	return 0;
+}
+
 /*
  * This should really be OPTION_FILENAME type as a part of
  * parse_options that take prefix to do this while parsing.
diff --git a/parse-options.h b/parse-options.h
index 5199950..034162e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -150,9 +150,15 @@ extern int parse_options_end(struct parse_opt_ctx_t *ctx);
 /*----- some often used options -----*/
 extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
 extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
+extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var)  OPT_BOOLEAN('v', "verbose", (var), "be verbose")
 #define OPT__QUIET(var)    OPT_BOOLEAN('q', "quiet",   (var), "be quiet")
+#define OPT__VERBOSITY(var) \
+	{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, "be more verbose", \
+	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
+	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, "be more quiet", \
+	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
 #define OPT__DRY_RUN(var)  OPT_BOOLEAN('n', "dry-run", (var), "dry run")
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), "n", \
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
new file mode 100755
index 0000000..83e2e8a
--- /dev/null
+++ b/t/t5521-pull-options.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='pull options'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+test_expect_success 'setup' '
+	mkdir parent &&
+	(cd parent && git init &&
+	 echo one >file && git add file &&
+	 git commit -m one)
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q' '
+	mkdir clonedq &&
+	cd clonedq &&
+	git pull -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull' '
+	mkdir cloned &&
+	cd cloned &&
+	git pull "$D/parent" >out 2>err &&
+	test -s out
+'
+cd "$D"
+
+test_expect_success 'git pull -v' '
+	mkdir clonedv &&
+	cd clonedv &&
+	git pull -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -v -q' '
+	mkdir clonedvq &&
+	cd clonedvq &&
+	git pull -v -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q -v' '
+	mkdir clonedqv &&
+	cd clonedqv &&
+	git pull -q -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+test_done
-- 
1.6.0.2.GIT

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15 17:42   ` Tuncer Ayaz
@ 2008-11-15 19:16     ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15 19:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sat, Nov 15, 2008 at 6:42 PM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> On Sat, Nov 15, 2008 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>>>               remote = remote_get(argv[0]);
>>>
>>>       transport = transport_get(remote, remote->url[0]);
>>> -     if (verbose >= 2)
>>> +     if (verbosity == VERBOSE)
>>>               transport->verbose = 1;
>>> -     if (quiet)
>>> +     if (verbosity == QUIET)
>>>               transport->verbose = -1;
>>>       if (upload_pack)
>>>               set_option(TRANS_OPT_UPLOADPACK, upload_pack);
>>
>> In the original code, the variable verbose can be ">= 2" when "-v -v" is
>> given, so transport->verbose is not turned on with a single "-v" alone
>> (this correctly mimics the original behaviour in the scripted version).
>>
>
> Doesn't this also mean that we need to be able to concatenate
> multiple -v options in git-pull.sh similar to what one of the first
> versions of my patch did?
>
> <snip>
>

The next patch revision arriving here in a minute or two
will do the following in git-pull.sh:
1) if -q then verbosity=-q
2) if -v then verbosity=verbosity + -v

This means that git pull -v -q will reset verbosity to -q
and allow counting of -v instances. If this is not desired
or incorrect another change is needed :-).

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15  1:15 ` Junio C Hamano
  2008-11-15  1:53   ` Tuncer Ayaz
@ 2008-11-15 17:42   ` Tuncer Ayaz
  2008-11-15 19:16     ` Tuncer Ayaz
  1 sibling, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15 17:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sat, Nov 15, 2008 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>>               remote = remote_get(argv[0]);
>>
>>       transport = transport_get(remote, remote->url[0]);
>> -     if (verbose >= 2)
>> +     if (verbosity == VERBOSE)
>>               transport->verbose = 1;
>> -     if (quiet)
>> +     if (verbosity == QUIET)
>>               transport->verbose = -1;
>>       if (upload_pack)
>>               set_option(TRANS_OPT_UPLOADPACK, upload_pack);
>
> In the original code, the variable verbose can be ">= 2" when "-v -v" is
> given, so transport->verbose is not turned on with a single "-v" alone
> (this correctly mimics the original behaviour in the scripted version).
>

Doesn't this also mean that we need to be able to concatenate
multiple -v options in git-pull.sh similar to what one of the first
versions of my patch did?

<snip>

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15  3:10     ` Junio C Hamano
@ 2008-11-15 17:28       ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15 17:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sat, Nov 15, 2008 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>>> The approach with enum { Q, N, V } cannot express this, unfortunately.
>>>
>>> So let's do something like the attached patch, instead.
>>
>> That's ok. We should not break anything.
>>
>> Do you want me to resubmit a new patch with the changes?
>
> As long as you are happy with the suggested change, there is no need to.
> You'd need to sign-off the patch, though...
>

I forgot to add -s to my commit call this time around.
I will apply your changes and repost it with the signoff.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15  1:53   ` Tuncer Ayaz
@ 2008-11-15  3:10     ` Junio C Hamano
  2008-11-15 17:28       ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2008-11-15  3:10 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: Junio C Hamano, git

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

>> The approach with enum { Q, N, V } cannot express this, unfortunately.
>>
>> So let's do something like the attached patch, instead.
>
> That's ok. We should not break anything.
>
> Do you want me to resubmit a new patch with the changes?

As long as you are happy with the suggested change, there is no need to.
You'd need to sign-off the patch, though...

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15  1:15 ` Junio C Hamano
@ 2008-11-15  1:53   ` Tuncer Ayaz
  2008-11-15  3:10     ` Junio C Hamano
  2008-11-15 17:42   ` Tuncer Ayaz
  1 sibling, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15  1:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sat, Nov 15, 2008 at 2:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>>               remote = remote_get(argv[0]);
>>
>>       transport = transport_get(remote, remote->url[0]);
>> -     if (verbose >= 2)
>> +     if (verbosity == VERBOSE)
>>               transport->verbose = 1;
>> -     if (quiet)
>> +     if (verbosity == QUIET)
>>               transport->verbose = -1;
>>       if (upload_pack)
>>               set_option(TRANS_OPT_UPLOADPACK, upload_pack);
>
> In the original code, the variable verbose can be ">= 2" when "-v -v" is
> given, so transport->verbose is not turned on with a single "-v" alone
> (this correctly mimics the original behaviour in the scripted version).
>
> The approach with enum { Q, N, V } cannot express this, unfortunately.
>
> So let's do something like the attached patch, instead.

That's ok. We should not break anything.

Do you want me to resubmit a new patch with the changes?

> The patch adds OPT__VERBOSITY() that allows you to say "-v -v" to increase
> verbosity (and "-q -q" to make it really quiet, although we do not use it
> anywhere yet).
>
>  builtin-fetch.c |   19 +++++++++----------
>  builtin-merge.c |   21 ++++++++++++++-------
>  parse-options.c |   22 ++++++++++++++++++++++
>  parse-options.h |    6 ++++++
>  4 files changed, 51 insertions(+), 17 deletions(-)
>
> diff --git c/builtin-fetch.c w/builtin-fetch.c
> index f151cfa..7568163 100644
> --- c/builtin-fetch.c
> +++ w/builtin-fetch.c
> @@ -22,7 +22,7 @@ enum {
>        TAGS_SET = 2
>  };
>
> -static int append, force, keep, update_head_ok, verbose, quiet;
> +static int append, force, keep, update_head_ok, verbosity;
>  static int tags = TAGS_DEFAULT;
>  static const char *depth;
>  static const char *upload_pack;
> @@ -30,8 +30,7 @@ static struct strbuf default_rla = STRBUF_INIT;
>  static struct transport *transport;
>
>  static struct option builtin_fetch_options[] = {
> -       OPT__QUIET(&quiet),
> -       OPT__VERBOSE(&verbose),
> +       OPT__VERBOSITY(&verbosity),
>        OPT_BOOLEAN('a', "append", &append,
>                    "append to .git/FETCH_HEAD instead of overwriting"),
>        OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
> @@ -192,7 +191,6 @@ static int s_update_ref(const char *action,
>
>  static int update_local_ref(struct ref *ref,
>                            const char *remote,
> -                           int verbose,
>                            char *display)
>  {
>        struct commit *current = NULL, *updated;
> @@ -210,7 +208,7 @@ static int update_local_ref(struct ref *ref,
>                die("object %s not found", sha1_to_hex(ref->new_sha1));
>
>        if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
> -               if (verbose)
> +               if (verbosity > 0)
>                        sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
>                                "[up to date]", REFCOL_WIDTH, remote,
>                                pretty_ref);
> @@ -366,18 +364,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
>                        note);
>
>                if (ref)
> -                       rc |= update_local_ref(ref, what, verbose, note);
> +                       rc |= update_local_ref(ref, what, note);
>                else
>                        sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
>                                SUMMARY_WIDTH, *kind ? kind : "branch",
>                                 REFCOL_WIDTH, *what ? what : "HEAD");
>                if (*note) {
> -                       if (!shown_url) {
> +                       if (verbosity >= 0 && !shown_url) {
>                                fprintf(stderr, "From %.*s\n",
>                                                url_len, url);
>                                shown_url = 1;
>                        }
> -                       fprintf(stderr, " %s\n", note);
> +                       if (verbosity >= 0)
> +                               fprintf(stderr, " %s\n", note);
>                }
>        }
>        fclose(fp);
> @@ -637,9 +636,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>                remote = remote_get(argv[0]);
>
>        transport = transport_get(remote, remote->url[0]);
> -       if (verbose >= 2)
> +       if (verbosity >= 2)
>                transport->verbose = 1;
> -       if (quiet)
> +       if (verbosity < 0)
>                transport->verbose = -1;
>        if (upload_pack)
>                set_option(TRANS_OPT_UPLOADPACK, upload_pack);
> diff --git c/builtin-merge.c w/builtin-merge.c
> index 5e7910b..7c2b90c 100644
> --- c/builtin-merge.c
> +++ w/builtin-merge.c
> @@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
>  static struct strategy **use_strategies;
>  static size_t use_strategies_nr, use_strategies_alloc;
>  static const char *branch;
> +static int verbosity;
>
>  static struct strategy all_strategy[] = {
>        { "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
> @@ -171,6 +172,7 @@ static struct option builtin_merge_options[] = {
>        OPT_CALLBACK('m', "message", &merge_msg, "message",
>                "message to be used for the merge commit (if any)",
>                option_parse_message),
> +       OPT__VERBOSITY(&verbosity),
>        OPT_END()
>  };
>
> @@ -250,7 +252,8 @@ static void restore_state(void)
>  /* This is called when no merge was necessary. */
>  static void finish_up_to_date(const char *msg)
>  {
> -       printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
> +       if (verbosity >= 0)
> +               printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
>        drop_save();
>  }
>
> @@ -331,14 +334,15 @@ static void finish(const unsigned char *new_head, const char *msg)
>        if (!msg)
>                strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
>        else {
> -               printf("%s\n", msg);
> +               if (verbosity >= 0)
> +                       printf("%s\n", msg);
>                strbuf_addf(&reflog_message, "%s: %s",
>                        getenv("GIT_REFLOG_ACTION"), msg);
>        }
>        if (squash) {
>                squash_message();
>        } else {
> -               if (!merge_msg.len)
> +               if (verbosity >= 0 && !merge_msg.len)
>                        printf("No merge message -- not updating HEAD\n");
>                else {
>                        const char *argv_gc_auto[] = { "gc", "--auto", NULL };
> @@ -872,6 +876,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
>        argc = parse_options(argc, argv, builtin_merge_options,
>                        builtin_merge_usage, 0);
> +       if (verbosity < 0)
> +               show_diffstat = 0;
>
>        if (squash) {
>                if (!allow_fast_forward)
> @@ -1013,10 +1019,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
>                strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
>
> -               printf("Updating %s..%s\n",
> -                       hex,
> -                       find_unique_abbrev(remoteheads->item->object.sha1,
> -                       DEFAULT_ABBREV));
> +               if (verbosity >= 0)
> +                       printf("Updating %s..%s\n",
> +                               hex,
> +                               find_unique_abbrev(remoteheads->item->object.sha1,
> +                               DEFAULT_ABBREV));
>                strbuf_addstr(&msg, "Fast forward");
>                if (have_message)
>                        strbuf_addstr(&msg,
> diff --git c/parse-options.c w/parse-options.c
> index fd08bb4..9eb55cc 100644
> --- c/parse-options.c
> +++ w/parse-options.c
> @@ -484,6 +484,28 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
>        return 0;
>  }
>
> +int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
> +                          int unset)
> +{
> +       int *target = opt->value;
> +
> +       if (unset)
> +               /* --no-quiet, --no-verbose */
> +               *target = 0;
> +       else if (opt->short_name == 'v') {
> +               if (*target >= 0)
> +                       (*target)++;
> +               else
> +                       *target = 1;
> +       } else {
> +               if (*target <= 0)
> +                       (*target)--;
> +               else
> +                       *target = -1;
> +       }
> +       return 0;
> +}
> +
>  /*
>  * This should really be OPTION_FILENAME type as a part of
>  * parse_options that take prefix to do this while parsing.
> diff --git c/parse-options.h w/parse-options.h
> index 5199950..034162e 100644
> --- c/parse-options.h
> +++ w/parse-options.h
> @@ -150,9 +150,15 @@ extern int parse_options_end(struct parse_opt_ctx_t *ctx);
>  /*----- some often used options -----*/
>  extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
>  extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
> +extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
>
>  #define OPT__VERBOSE(var)  OPT_BOOLEAN('v', "verbose", (var), "be verbose")
>  #define OPT__QUIET(var)    OPT_BOOLEAN('q', "quiet",   (var), "be quiet")
> +#define OPT__VERBOSITY(var) \
> +       { OPTION_CALLBACK, 'v', "verbose", (var), NULL, "be more verbose", \
> +         PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
> +       { OPTION_CALLBACK, 'q', "quiet", (var), NULL, "be more quiet", \
> +         PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
>  #define OPT__DRY_RUN(var)  OPT_BOOLEAN('n', "dry-run", (var), "dry run")
>  #define OPT__ABBREV(var)  \
>        { OPTION_CALLBACK, 0, "abbrev", (var), "n", \
>

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-15  0:14 Tuncer Ayaz
@ 2008-11-15  1:15 ` Junio C Hamano
  2008-11-15  1:53   ` Tuncer Ayaz
  2008-11-15 17:42   ` Tuncer Ayaz
  0 siblings, 2 replies; 29+ messages in thread
From: Junio C Hamano @ 2008-11-15  1:15 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: git

> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  		remote = remote_get(argv[0]);
>  
>  	transport = transport_get(remote, remote->url[0]);
> -	if (verbose >= 2)
> +	if (verbosity == VERBOSE)
>  		transport->verbose = 1;
> -	if (quiet)
> +	if (verbosity == QUIET)
>  		transport->verbose = -1;
>  	if (upload_pack)
>  		set_option(TRANS_OPT_UPLOADPACK, upload_pack);

In the original code, the variable verbose can be ">= 2" when "-v -v" is
given, so transport->verbose is not turned on with a single "-v" alone
(this correctly mimics the original behaviour in the scripted version).

The approach with enum { Q, N, V } cannot express this, unfortunately.

So let's do something like the attached patch, instead.

The patch adds OPT__VERBOSITY() that allows you to say "-v -v" to increase
verbosity (and "-q -q" to make it really quiet, although we do not use it
anywhere yet).

 builtin-fetch.c |   19 +++++++++----------
 builtin-merge.c |   21 ++++++++++++++-------
 parse-options.c |   22 ++++++++++++++++++++++
 parse-options.h |    6 ++++++
 4 files changed, 51 insertions(+), 17 deletions(-)

diff --git c/builtin-fetch.c w/builtin-fetch.c
index f151cfa..7568163 100644
--- c/builtin-fetch.c
+++ w/builtin-fetch.c
@@ -22,7 +22,7 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok, verbosity;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
@@ -30,8 +30,7 @@ static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT__VERBOSITY(&verbosity),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +191,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +208,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity > 0)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +364,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity >= 0 && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity >= 0)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -637,9 +636,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity >= 2)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity < 0)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git c/builtin-merge.c w/builtin-merge.c
index 5e7910b..7c2b90c 100644
--- c/builtin-merge.c
+++ w/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static int verbosity;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -171,6 +172,7 @@ static struct option builtin_merge_options[] = {
 	OPT_CALLBACK('m', "message", &merge_msg, "message",
 		"message to be used for the merge commit (if any)",
 		option_parse_message),
+	OPT__VERBOSITY(&verbosity),
 	OPT_END()
 };
 
@@ -250,7 +252,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity >= 0)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +334,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity >= 0)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity >= 0 && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +876,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity < 0)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1019,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity >= 0)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git c/parse-options.c w/parse-options.c
index fd08bb4..9eb55cc 100644
--- c/parse-options.c
+++ w/parse-options.c
@@ -484,6 +484,28 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
 	return 0;
 }
 
+int parse_opt_verbosity_cb(const struct option *opt, const char *arg,
+			   int unset)
+{
+	int *target = opt->value;
+
+	if (unset)
+		/* --no-quiet, --no-verbose */
+		*target = 0;
+	else if (opt->short_name == 'v') {
+		if (*target >= 0)
+			(*target)++;
+		else
+			*target = 1;
+	} else {
+		if (*target <= 0)
+			(*target)--;
+		else
+			*target = -1;
+	}
+	return 0;
+}
+
 /*
  * This should really be OPTION_FILENAME type as a part of
  * parse_options that take prefix to do this while parsing.
diff --git c/parse-options.h w/parse-options.h
index 5199950..034162e 100644
--- c/parse-options.h
+++ w/parse-options.h
@@ -150,9 +150,15 @@ extern int parse_options_end(struct parse_opt_ctx_t *ctx);
 /*----- some often used options -----*/
 extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
 extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
+extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
 
 #define OPT__VERBOSE(var)  OPT_BOOLEAN('v', "verbose", (var), "be verbose")
 #define OPT__QUIET(var)    OPT_BOOLEAN('q', "quiet",   (var), "be quiet")
+#define OPT__VERBOSITY(var) \
+	{ OPTION_CALLBACK, 'v', "verbose", (var), NULL, "be more verbose", \
+	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }, \
+	{ OPTION_CALLBACK, 'q', "quiet", (var), NULL, "be more quiet", \
+	  PARSE_OPT_NOARG, &parse_opt_verbosity_cb, 0 }
 #define OPT__DRY_RUN(var)  OPT_BOOLEAN('n', "dry-run", (var), "dry run")
 #define OPT__ABBREV(var)  \
 	{ OPTION_CALLBACK, 0, "abbrev", (var), "n", \

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

* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-11-15  0:14 Tuncer Ayaz
  2008-11-15  1:15 ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15  0:14 UTC (permalink / raw)
  To: git; +Cc: gitster

Implement git-pull --quiet and git-pull --verbose by
adding the options to git-pull and fixing verbosity
handling in git-fetch.
---
 Documentation/merge-options.txt |    8 +++++
 builtin-fetch.c                 |   21 +++++++------
 builtin-merge.c                 |   22 ++++++++++----
 git-pull.sh                     |   10 ++++--
 t/t5521-pull-options.sh         |   60 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+), 20 deletions(-)
 create mode 100755 t/t5521-pull-options.sh

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index f151cfa..efc5801 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,16 +22,17 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity == VERBOSE)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity > QUIET && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity > QUIET)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity == VERBOSE)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity == QUIET)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e7910b..a35f944 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
 }
 
 static struct option builtin_merge_options[] = {
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		"do not show a diffstat at the end of the merge",
 		PARSE_OPT_NOARG, option_parse_n },
@@ -250,7 +253,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity > QUIET)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity > QUIET)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity > QUIET && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity == QUIET)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity > QUIET)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 664fe34..8866f2a 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,13 +16,17 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
+strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity=-q ;;
+	-v|--verbose)
+		verbosity=-v ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +125,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"
@@ -182,4 +186,4 @@ test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
 exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head
+	"$merge_name" HEAD $merge_head $verbosity
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
new file mode 100755
index 0000000..83e2e8a
--- /dev/null
+++ b/t/t5521-pull-options.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='pull options'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+test_expect_success 'setup' '
+	mkdir parent &&
+	(cd parent && git init &&
+	 echo one >file && git add file &&
+	 git commit -m one)
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q' '
+	mkdir clonedq &&
+	cd clonedq &&
+	git pull -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull' '
+	mkdir cloned &&
+	cd cloned &&
+	git pull "$D/parent" >out 2>err &&
+	test -s out
+'
+cd "$D"
+
+test_expect_success 'git pull -v' '
+	mkdir clonedv &&
+	cd clonedv &&
+	git pull -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -v -q' '
+	mkdir clonedvq &&
+	cd clonedvq &&
+	git pull -v -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q -v' '
+	mkdir clonedqv &&
+	cd clonedqv &&
+	git pull -q -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+test_done
-- 
1.6.0.4

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-12 20:47   ` Junio C Hamano
@ 2008-11-15  0:09     ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-15  0:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Nov 12, 2008 at 9:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>> On Fri, Nov 7, 2008 at 4:26 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
>>> Implement git-pull --quiet and git-pull --verbose by
>>> adding the options to git-pull and fixing verbosity
>>> handling in git-fetch.
>>
>> Junio,
>>
>> is there anything still missing in this patch?
>> Maybe the name of the test-case is bad.
>
> It seems to break t7600.

thanks for the good review, it sure does break.

I hadn't noticed it as I didn't compare test results
of master against my branch and assumed that that
breakage is part of common failing tests.

Shawn helped me out a little bit with debugging the test scripts
and I quickly found out that an if clause is wrong.
the diff for the last patch to correct it is:
 --- a/builtin-merge.c
 +++ b/builtin-merge.c
 @@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
@@ -163,7 +144,7 @@

        argc = parse_options(argc, argv, builtin_merge_options,
                        builtin_merge_usage, 0);
-+      if (verbosity > QUIET)
++      if (verbosity == QUIET)
 +              show_diffstat = 0;


a corrected patch will arrive here in a minute.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-10 23:43 ` Tuncer Ayaz
@ 2008-11-12 20:47   ` Junio C Hamano
  2008-11-15  0:09     ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2008-11-12 20:47 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: git

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

> On Fri, Nov 7, 2008 at 4:26 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
>> Implement git-pull --quiet and git-pull --verbose by
>> adding the options to git-pull and fixing verbosity
>> handling in git-fetch.
>
> Junio,
>
> is there anything still missing in this patch?
> Maybe the name of the test-case is bad.

It seems to break t7600.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-11-07  3:26 Tuncer Ayaz
@ 2008-11-10 23:43 ` Tuncer Ayaz
  2008-11-12 20:47   ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-10 23:43 UTC (permalink / raw)
  To: git, Junio C Hamano

On Fri, Nov 7, 2008 at 4:26 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> Implement git-pull --quiet and git-pull --verbose by
> adding the options to git-pull and fixing verbosity
> handling in git-fetch.

Junio,

is there anything still missing in this patch?
Maybe the name of the test-case is bad.
Maybe you've queued it for review already.

Regards,

        Tuncer Ayaz

> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
>  Documentation/merge-options.txt |    8 +++++
>  builtin-fetch.c                 |   21 +++++++------
>  builtin-merge.c                 |   22 ++++++++++----
>  git-pull.sh                     |   10 ++++--
>  t/t5521-pull-options.sh         |   60 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 101 insertions(+), 20 deletions(-)
>  create mode 100755 t/t5521-pull-options.sh
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 007909a..427cdef 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -1,3 +1,11 @@
> +-q::
> +--quiet::
> +       Operate quietly.
> +
> +-v::
> +--verbose::
> +       Be verbose.
> +
>  --stat::
>        Show a diffstat at the end of the merge. The diffstat is also
>        controlled by the configuration option merge.stat.
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index f151cfa..efc5801 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -22,16 +22,17 @@ enum {
>        TAGS_SET = 2
>  };
>
> -static int append, force, keep, update_head_ok, verbose, quiet;
> +static int append, force, keep, update_head_ok;
>  static int tags = TAGS_DEFAULT;
>  static const char *depth;
>  static const char *upload_pack;
>  static struct strbuf default_rla = STRBUF_INIT;
>  static struct transport *transport;
> +static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
>
>  static struct option builtin_fetch_options[] = {
> -       OPT__QUIET(&quiet),
> -       OPT__VERBOSE(&verbose),
> +       OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
> +       OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
>        OPT_BOOLEAN('a', "append", &append,
>                    "append to .git/FETCH_HEAD instead of overwriting"),
>        OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
> @@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
>
>  static int update_local_ref(struct ref *ref,
>                            const char *remote,
> -                           int verbose,
>                            char *display)
>  {
>        struct commit *current = NULL, *updated;
> @@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
>                die("object %s not found", sha1_to_hex(ref->new_sha1));
>
>        if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
> -               if (verbose)
> +               if (verbosity == VERBOSE)
>                        sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
>                                "[up to date]", REFCOL_WIDTH, remote,
>                                pretty_ref);
> @@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
>                        note);
>
>                if (ref)
> -                       rc |= update_local_ref(ref, what, verbose, note);
> +                       rc |= update_local_ref(ref, what, note);
>                else
>                        sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
>                                SUMMARY_WIDTH, *kind ? kind : "branch",
>                                 REFCOL_WIDTH, *what ? what : "HEAD");
>                if (*note) {
> -                       if (!shown_url) {
> +                       if (verbosity > QUIET && !shown_url) {
>                                fprintf(stderr, "From %.*s\n",
>                                                url_len, url);
>                                shown_url = 1;
>                        }
> -                       fprintf(stderr, " %s\n", note);
> +                       if (verbosity > QUIET)
> +                               fprintf(stderr, " %s\n", note);
>                }
>        }
>        fclose(fp);
> @@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>                remote = remote_get(argv[0]);
>
>        transport = transport_get(remote, remote->url[0]);
> -       if (verbose >= 2)
> +       if (verbosity == VERBOSE)
>                transport->verbose = 1;
> -       if (quiet)
> +       if (verbosity == QUIET)
>                transport->verbose = -1;
>        if (upload_pack)
>                set_option(TRANS_OPT_UPLOADPACK, upload_pack);
> diff --git a/builtin-merge.c b/builtin-merge.c
> index 5e7910b..76e2890 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
>  static struct strategy **use_strategies;
>  static size_t use_strategies_nr, use_strategies_alloc;
>  static const char *branch;
> +static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
>
>  static struct strategy all_strategy[] = {
>        { "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
> @@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
>  }
>
>  static struct option builtin_merge_options[] = {
> +       OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
> +       OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
>        { OPTION_CALLBACK, 'n', NULL, NULL, NULL,
>                "do not show a diffstat at the end of the merge",
>                PARSE_OPT_NOARG, option_parse_n },
> @@ -250,7 +253,8 @@ static void restore_state(void)
>  /* This is called when no merge was necessary. */
>  static void finish_up_to_date(const char *msg)
>  {
> -       printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
> +       if (verbosity > QUIET)
> +               printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
>        drop_save();
>  }
>
> @@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
>        if (!msg)
>                strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
>        else {
> -               printf("%s\n", msg);
> +               if (verbosity > QUIET)
> +                       printf("%s\n", msg);
>                strbuf_addf(&reflog_message, "%s: %s",
>                        getenv("GIT_REFLOG_ACTION"), msg);
>        }
>        if (squash) {
>                squash_message();
>        } else {
> -               if (!merge_msg.len)
> +               if (verbosity > QUIET && !merge_msg.len)
>                        printf("No merge message -- not updating HEAD\n");
>                else {
>                        const char *argv_gc_auto[] = { "gc", "--auto", NULL };
> @@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
>        argc = parse_options(argc, argv, builtin_merge_options,
>                        builtin_merge_usage, 0);
> +       if (verbosity > QUIET)
> +               show_diffstat = 0;
>
>        if (squash) {
>                if (!allow_fast_forward)
> @@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>
>                strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
>
> -               printf("Updating %s..%s\n",
> -                       hex,
> -                       find_unique_abbrev(remoteheads->item->object.sha1,
> -                       DEFAULT_ABBREV));
> +               if (verbosity > QUIET)
> +                       printf("Updating %s..%s\n",
> +                               hex,
> +                               find_unique_abbrev(remoteheads->item->object.sha1,
> +                               DEFAULT_ABBREV));
>                strbuf_addstr(&msg, "Fast forward");
>                if (have_message)
>                        strbuf_addstr(&msg,
> diff --git a/git-pull.sh b/git-pull.sh
> index 664fe34..8866f2a 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -16,13 +16,17 @@ cd_to_toplevel
>  test -z "$(git ls-files -u)" ||
>        die "You are in the middle of a conflicted merge."
>
> -strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
> +strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
>  curr_branch=$(git symbolic-ref -q HEAD)
>  curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
>  rebase=$(git config --bool branch.$curr_branch_short.rebase)
>  while :
>  do
>        case "$1" in
> +       -q|--quiet)
> +               verbosity=-q ;;
> +       -v|--verbose)
> +               verbosity=-v ;;
>        -n|--no-stat|--no-summary)
>                no_stat=-n ;;
>        --stat|--summary)
> @@ -121,7 +125,7 @@ test true = "$rebase" && {
>                "refs/remotes/$origin/$reflist" 2>/dev/null)"
>  }
>  orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
> -git fetch --update-head-ok "$@" || exit 1
> +git fetch $verbosity --update-head-ok "$@" || exit 1
>
>  curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
>  if test -n "$orig_head" && test "$curr_head" != "$orig_head"
> @@ -182,4 +186,4 @@ test true = "$rebase" &&
>        exec git-rebase $strategy_args --onto $merge_head \
>        ${oldremoteref:-$merge_head}
>  exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
> -       "$merge_name" HEAD $merge_head
> +       "$merge_name" HEAD $merge_head $verbosity
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> new file mode 100755
> index 0000000..83e2e8a
> --- /dev/null
> +++ b/t/t5521-pull-options.sh
> @@ -0,0 +1,60 @@
> +#!/bin/sh
> +
> +test_description='pull options'
> +
> +. ./test-lib.sh
> +
> +D=`pwd`
> +
> +test_expect_success 'setup' '
> +       mkdir parent &&
> +       (cd parent && git init &&
> +        echo one >file && git add file &&
> +        git commit -m one)
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -q' '
> +       mkdir clonedq &&
> +       cd clonedq &&
> +       git pull -q "$D/parent" >out 2>err &&
> +       test ! -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull' '
> +       mkdir cloned &&
> +       cd cloned &&
> +       git pull "$D/parent" >out 2>err &&
> +       test -s out
> +'
> +cd "$D"
> +
> +test_expect_success 'git pull -v' '
> +       mkdir clonedv &&
> +       cd clonedv &&
> +       git pull -v "$D/parent" >out 2>err &&
> +       test -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -v -q' '
> +       mkdir clonedvq &&
> +       cd clonedvq &&
> +       git pull -v -q "$D/parent" >out 2>err &&
> +       test ! -s out
> +'
> +
> +cd "$D"
> +
> +test_expect_success 'git pull -q -v' '
> +       mkdir clonedqv &&
> +       cd clonedqv &&
> +       git pull -q -v "$D/parent" >out 2>err &&
> +       test -s out
> +'
> +
> +test_done
> --
> 1.6.0.2.GIT
>
>

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

* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-11-07  3:26 Tuncer Ayaz
  2008-11-10 23:43 ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-11-07  3:26 UTC (permalink / raw)
  To: git

Implement git-pull --quiet and git-pull --verbose by
adding the options to git-pull and fixing verbosity
handling in git-fetch.

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 Documentation/merge-options.txt |    8 +++++
 builtin-fetch.c                 |   21 +++++++------
 builtin-merge.c                 |   22 ++++++++++----
 git-pull.sh                     |   10 ++++--
 t/t5521-pull-options.sh         |   60 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+), 20 deletions(-)
 create mode 100755 t/t5521-pull-options.sh

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index f151cfa..efc5801 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,16 +22,17 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity == VERBOSE)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity > QUIET && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity > QUIET)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -637,9 +638,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity == VERBOSE)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity == QUIET)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e7910b..76e2890 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
 }
 
 static struct option builtin_merge_options[] = {
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		"do not show a diffstat at the end of the merge",
 		PARSE_OPT_NOARG, option_parse_n },
@@ -250,7 +253,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity > QUIET)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity > QUIET)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity > QUIET && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity > QUIET)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity > QUIET)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 664fe34..8866f2a 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,13 +16,17 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
+strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity=-q ;;
+	-v|--verbose)
+		verbosity=-v ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +125,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test -n "$orig_head" && test "$curr_head" != "$orig_head"
@@ -182,4 +186,4 @@ test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
 exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
-	"$merge_name" HEAD $merge_head
+	"$merge_name" HEAD $merge_head $verbosity
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
new file mode 100755
index 0000000..83e2e8a
--- /dev/null
+++ b/t/t5521-pull-options.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='pull options'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+test_expect_success 'setup' '
+	mkdir parent &&
+	(cd parent && git init &&
+	 echo one >file && git add file &&
+	 git commit -m one)
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q' '
+	mkdir clonedq &&
+	cd clonedq &&
+	git pull -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull' '
+	mkdir cloned &&
+	cd cloned &&
+	git pull "$D/parent" >out 2>err &&
+	test -s out
+'
+cd "$D"
+
+test_expect_success 'git pull -v' '
+	mkdir clonedv &&
+	cd clonedv &&
+	git pull -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -v -q' '
+	mkdir clonedvq &&
+	cd clonedvq &&
+	git pull -v -q "$D/parent" >out 2>err &&
+	test ! -s out
+'
+
+cd "$D"
+
+test_expect_success 'git pull -q -v' '
+	mkdir clonedqv &&
+	cd clonedqv &&
+	git pull -q -v "$D/parent" >out 2>err &&
+	test -s out
+'
+
+test_done
-- 
1.6.0.2.GIT

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-20 23:54     ` Junio C Hamano
@ 2008-10-21 16:25       ` Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-10-21 16:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Tue, Oct 21, 2008 at 1:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:
>
>> On Sun, Oct 19, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>>> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>>>>  while :
>>>>  do
>>>>       case "$1" in
>>>> +     -q|--quiet)
>>>> +             verbosity="$verbosity -q" ;;
>>>> +     -v|--verbose)
>>>> +             verbosity="$verbosity -v" ;;
>>>
>>> You know verbosity flags (-q and -v) are "the last one wins", so I do not
>>> see much point in this concatenation.
>>
>> Without concatenation I would need to analyze the content
>> of the variable each time the option is passed to the shell
>> script. Do you know of a simpler/better way still keeping the
>> functionality that
>> $ git pull -q -v --quiet --verbose --quiet gives verbosity=QUIET
>> and
>> $ git pull -q -v --quiet --verbose --quiet -v yields verbosity=VERBOSE
>> ?
>
> Wouldn't
>
>        verbosity=
>        while :
>        do
>                case "$1" in
>                -q|--quiet) verbosity=-q ;;
>                -v|--verbose) verbosity=-v ;;
>                ... others ...
>                esac
>                shift
>        done
>        git pull $verbosity other options
>
> give the -q for the former and -v for the latter to "git pull"?

Yes that is much simpler and works :). Thanks.
Please see my next patch in a few minutes.
I might not reply before the weekend as I'm pretty busy, btw.

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-20 16:35   ` Tuncer Ayaz
@ 2008-10-20 23:54     ` Junio C Hamano
  2008-10-21 16:25       ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2008-10-20 23:54 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: Junio C Hamano, git

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

> On Sun, Oct 19, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>>> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>>>  while :
>>>  do
>>>       case "$1" in
>>> +     -q|--quiet)
>>> +             verbosity="$verbosity -q" ;;
>>> +     -v|--verbose)
>>> +             verbosity="$verbosity -v" ;;
>>
>> You know verbosity flags (-q and -v) are "the last one wins", so I do not
>> see much point in this concatenation.
>
> Without concatenation I would need to analyze the content
> of the variable each time the option is passed to the shell
> script. Do you know of a simpler/better way still keeping the
> functionality that
> $ git pull -q -v --quiet --verbose --quiet gives verbosity=QUIET
> and
> $ git pull -q -v --quiet --verbose --quiet -v yields verbosity=VERBOSE
> ?

Wouldn't

	verbosity=
	while :
        do
        	case "$1" in
                -q|--quiet) verbosity=-q ;;
                -v|--verbose) verbosity=-v ;;
		... others ...
                esac
                shift
	done
        git pull $verbosity other options

give the -q for the former and -v for the latter to "git pull"?

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-19 21:26 ` Junio C Hamano
@ 2008-10-20 16:35   ` Tuncer Ayaz
  2008-10-20 23:54     ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-10-20 16:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sun, Oct 19, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:
>
>> 2) my adaption of the following two lines from
>> builtin-fetch.c to the new verbosity option:
>>     if (verbosity == VERBOSE)
>>         transport->verbose = 1;
>>     if (verbosity == QUIET)
>>         transport->verbose = -1;
>
> Hmm, what's wrong with it?  Looks Ok to me...

Just wanted to be sure it's correct, that's all.

Actually I think the old code:
if (verbose >= 2)
    transport->verbose = 1;
is wrong and probably a leftover from old days
as Shawn confirmed.

>>  static struct option builtin_fetch_options[] = {
>> -     OPT__QUIET(&quiet),
>> -     OPT__VERBOSE(&verbose),
>> +     { OPTION_CALLBACK, 'q', "quiet", NULL, NULL,
>> +             "operate quietly",
>> +             PARSE_OPT_NOARG, option_parse_quiet },
>> +     { OPTION_CALLBACK, 'v', "verbose", NULL, NULL,
>> +             "be verbose",
>> +             PARSE_OPT_NOARG, option_parse_verbose },
>
> Isn't there a OPTION_FOO that assigns a constant to the given variable?

Yes, there is - OPT_SET_INT and I've used that in my next patch.

>> @@ -192,7 +211,6 @@ static int s_update_ref(const char *action,
>>
>>  static int update_local_ref(struct ref *ref,
>>                           const char *remote,
>> -                         int verbose,
>>                           char *display)
>>  {
>>       struct commit *current = NULL, *updated;
>> ...
>> @@ -366,18 +384,19 @@ static int store_updated_refs(const char *url, const char
>> *remote_name,
>>                       note);
>>
>>               if (ref)
>> -                     rc |= update_local_ref(ref, what, verbose, note);
>> +                     rc |= update_local_ref(ref, what, note);
>
> Hmph, in the existing code, do_fetch()->fetch_refs()->store_updated_refs()
> callchain relies on the "verbose" to be global anyway, so losing the
> ability to call update_local_ref() with verbosity as parameter is not a
> huge deal.

OK, I've kept the removal of the verbose param in the next patch.

> I however think it would be more beneficial in the longer term to keep
> "verbosity" as parameter so the caller can tweak what the callee does, and
> making large part of what cmd_fetch() does callable from outside.  That
> would involve making the builtin_fetch_options[] on-stack, and passing
> verbosity (and possibly other variables currently used as file-scope
> global) as parameters, which is outside of the scope of your patch, but it
> is something to keep in mind.
>
>> diff --git a/git-pull.sh b/git-pull.sh
>> index 75c3610..dc613db 100755
>> --- a/git-pull.sh
>> +++ b/git-pull.sh
>> @@ -16,6 +16,7 @@ cd_to_toplevel
>>  test -z "$(git ls-files -u)" ||
>>       die "You are in the middle of a conflicted merge."
>>
>> +verbosity=
>>  strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
>>  curr_branch=$(git symbolic-ref -q HEAD)
>>  curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
>
> It would fit at the end of the next line just fine, wouldn't it?
>
>> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>>  while :
>>  do
>>       case "$1" in
>> +     -q|--quiet)
>> +             verbosity="$verbosity -q" ;;
>> +     -v|--verbose)
>> +             verbosity="$verbosity -v" ;;
>
> You know verbosity flags (-q and -v) are "the last one wins", so I do not
> see much point in this concatenation.

Without concatenation I would need to analyze the content
of the variable each time the option is passed to the shell
script. Do you know of a simpler/better way still keeping the
functionality that
$ git pull -q -v --quiet --verbose --quiet gives verbosity=QUIET
and
$ git pull -q -v --quiet --verbose --quiet -v yields verbosity=VERBOSE
?

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

* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-10-20 16:28 Tuncer Ayaz
  0 siblings, 0 replies; 29+ messages in thread
From: Tuncer Ayaz @ 2008-10-20 16:28 UTC (permalink / raw)
  To: git; +Cc: gitster

After fixing clone -q I noticed that pull -q does not do what
it's supposed to do and implemented --quiet/--verbose by
adding it to builtin-merge and fixing two places in builtin-fetch.

I have not touched/adjusted contrib/completion/git-completion.bash
but can take a look if wanted. I think it already needs one or two
adjustments caused by recent --OPTIONS changes in master.

I've tested the following invocations with the below changes applied:
$ git pull
$ git pull -q
$ git pull -v
$ git fetch -q
$ git pull -q -v -q

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 Documentation/merge-options.txt |    8 ++++++++
 builtin-fetch.c                 |   21 +++++++++++----------
 builtin-merge.c                 |   22 +++++++++++++++-------
 git-pull.sh                     |   11 ++++++++---
 4 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index ee93d3a..b067512 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,16 +22,17 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +193,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +210,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity == VERBOSE)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +366,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity > QUIET && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity > QUIET)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -622,9 +623,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity == VERBOSE)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity == QUIET)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e7910b..76e2890 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -152,6 +153,8 @@ static int option_parse_n(const struct option *opt,
 }
 
 static struct option builtin_merge_options[] = {
+	OPT_SET_INT('v', "verbose", &verbosity, "be verbose", VERBOSE),
+	OPT_SET_INT('q', "quiet", &verbosity, "operate quietly", QUIET),
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		"do not show a diffstat at the end of the merge",
 		PARSE_OPT_NOARG, option_parse_n },
@@ -250,7 +253,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity > QUIET)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -331,14 +335,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity > QUIET)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity > QUIET && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -872,6 +877,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity > QUIET)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1013,10 +1020,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity > QUIET)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 75c3610..9c0a812 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,13 +16,17 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
-strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
+strategy_args= no_stat= no_commit= squash= no_ff= log_arg= verbosity=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity="$verbosity -q" ;;
+	-v|--verbose)
+		verbosity="$verbosity -v" ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +125,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test "$curr_head" != "$orig_head"
@@ -181,5 +185,6 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
+exec git-merge $verbosity $no_stat $no_commit \
+	$squash $no_ff $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head
-- 
1.6.0.2.GIT

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

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
  2008-10-19 19:48 Tuncer Ayaz
@ 2008-10-19 21:26 ` Junio C Hamano
  2008-10-20 16:35   ` Tuncer Ayaz
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2008-10-19 21:26 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: git

Tuncer Ayaz <tuncer.ayaz@gmail.com> writes:

> 2) my adaption of the following two lines from
> builtin-fetch.c to the new verbosity option:
>     if (verbosity == VERBOSE)
>         transport->verbose = 1;
>     if (verbosity == QUIET)
>         transport->verbose = -1;

Hmm, what's wrong with it?  Looks Ok to me...

>  static struct option builtin_fetch_options[] = {
> -	OPT__QUIET(&quiet),
> -	OPT__VERBOSE(&verbose),
> +	{ OPTION_CALLBACK, 'q', "quiet", NULL, NULL,
> +		"operate quietly",
> +		PARSE_OPT_NOARG, option_parse_quiet },
> +	{ OPTION_CALLBACK, 'v', "verbose", NULL, NULL,
> +		"be verbose",
> +		PARSE_OPT_NOARG, option_parse_verbose },

Isn't there a OPTION_FOO that assigns a constant to the given variable?

> @@ -192,7 +211,6 @@ static int s_update_ref(const char *action,
>  
>  static int update_local_ref(struct ref *ref,
>  			    const char *remote,
> -			    int verbose,
>  			    char *display)
>  {
>  	struct commit *current = NULL, *updated;
> ...
> @@ -366,18 +384,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
>  			note);
>  
>  		if (ref)
> -			rc |= update_local_ref(ref, what, verbose, note);
> +			rc |= update_local_ref(ref, what, note);

Hmph, in the existing code, do_fetch()->fetch_refs()->store_updated_refs()
callchain relies on the "verbose" to be global anyway, so losing the
ability to call update_local_ref() with verbosity as parameter is not a
huge deal.

I however think it would be more beneficial in the longer term to keep
"verbosity" as parameter so the caller can tweak what the callee does, and
making large part of what cmd_fetch() does callable from outside.  That
would involve making the builtin_fetch_options[] on-stack, and passing
verbosity (and possibly other variables currently used as file-scope
global) as parameters, which is outside of the scope of your patch, but it
is something to keep in mind.

> diff --git a/git-pull.sh b/git-pull.sh
> index 75c3610..dc613db 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -16,6 +16,7 @@ cd_to_toplevel
>  test -z "$(git ls-files -u)" ||
>  	die "You are in the middle of a conflicted merge."
>  
> +verbosity=
>  strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
>  curr_branch=$(git symbolic-ref -q HEAD)
>  curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")

It would fit at the end of the next line just fine, wouldn't it?

> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>  while :
>  do
>  	case "$1" in
> +	-q|--quiet)
> +		verbosity="$verbosity -q" ;;
> +	-v|--verbose)
> +		verbosity="$verbosity -v" ;;

You know verbosity flags (-q and -v) are "the last one wins", so I do not
see much point in this concatenation.

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

* [PATCH] Teach/Fix pull/fetch -q/-v options
@ 2008-10-19 19:48 Tuncer Ayaz
  2008-10-19 21:26 ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Tuncer Ayaz @ 2008-10-19 19:48 UTC (permalink / raw)
  To: git; +Cc: gitster

After fixing clone -q I noticed that pull -q does not do what
it's supposed to do and implemented --quiet/--verbose by
adding it to builtin-merge and fixing two places in builtin-fetch.

I have not touched/adjusted contrib/completion/git-completion.bash
but can take a look if wanted. I think it already needs one or two
adjustments caused by recent --OPTIONS changes in master.

I've tested the following invocations with the below changes applied:
$ git pull
$ git pull -q
$ git pull -v

This is the next attempt trying to incorporate Junio's
suggestions and I'm not sure about the following:
1) having the same option callback function in both modules
2) my adaption of the following two lines from
builtin-fetch.c to the new verbosity option:
    if (verbosity == VERBOSE)
        transport->verbose = 1;
    if (verbosity == QUIET)
        transport->verbose = -1;
3) my usage of OPTION_CALLBACK
4) my support for overriding -q or -v
as implemented in git-pull.sh by appending
-q or -v as it is passed to the script

therefore please correct me if I did it wrong or my
cb fun has an obvious defect. it may very well
have one :)

This revision does actually override verbosity
based on what was supplied later in argv (-q or -v).

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 Documentation/merge-options.txt |    8 +++++++
 builtin-fetch.c                 |   39 ++++++++++++++++++++++++++++---------
 builtin-merge.c                 |   40 ++++++++++++++++++++++++++++++++------
 git-pull.sh                     |   10 +++++++-
 4 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 007909a..427cdef 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -1,3 +1,11 @@
+-q::
+--quiet::
+	Operate quietly.
+
+-v::
+--verbose::
+	Be verbose.
+
 --stat::
 	Show a diffstat at the end of the merge. The diffstat is also
 	controlled by the configuration option merge.stat.
diff --git a/builtin-fetch.c b/builtin-fetch.c
index ee93d3a..2596aee 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -22,16 +22,35 @@ enum {
 	TAGS_SET = 2
 };
 
-static int append, force, keep, update_head_ok, verbose, quiet;
+static int append, force, keep, update_head_ok;
 static int tags = TAGS_DEFAULT;
 static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *transport;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
+
+static int option_parse_quiet(const struct option *opt,
+			  const char *arg, int unset)
+{
+	verbosity = QUIET;
+	return 0;
+}
+
+static int option_parse_verbose(const struct option *opt,
+			  const char *arg, int unset)
+{
+	verbosity = VERBOSE;
+	return 0;
+}
 
 static struct option builtin_fetch_options[] = {
-	OPT__QUIET(&quiet),
-	OPT__VERBOSE(&verbose),
+	{ OPTION_CALLBACK, 'q', "quiet", NULL, NULL,
+		"operate quietly",
+		PARSE_OPT_NOARG, option_parse_quiet },
+	{ OPTION_CALLBACK, 'v', "verbose", NULL, NULL,
+		"be verbose",
+		PARSE_OPT_NOARG, option_parse_verbose },
 	OPT_BOOLEAN('a', "append", &append,
 		    "append to .git/FETCH_HEAD instead of overwriting"),
 	OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -192,7 +211,6 @@ static int s_update_ref(const char *action,
 
 static int update_local_ref(struct ref *ref,
 			    const char *remote,
-			    int verbose,
 			    char *display)
 {
 	struct commit *current = NULL, *updated;
@@ -210,7 +228,7 @@ static int update_local_ref(struct ref *ref,
 		die("object %s not found", sha1_to_hex(ref->new_sha1));
 
 	if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
-		if (verbose)
+		if (verbosity == VERBOSE)
 			sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
 				"[up to date]", REFCOL_WIDTH, remote,
 				pretty_ref);
@@ -366,18 +384,19 @@ static int store_updated_refs(const char *url, const char *remote_name,
 			note);
 
 		if (ref)
-			rc |= update_local_ref(ref, what, verbose, note);
+			rc |= update_local_ref(ref, what, note);
 		else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");
 		if (*note) {
-			if (!shown_url) {
+			if (verbosity > QUIET && !shown_url) {
 				fprintf(stderr, "From %.*s\n",
 						url_len, url);
 				shown_url = 1;
 			}
-			fprintf(stderr, " %s\n", note);
+			if (verbosity > QUIET)
+				fprintf(stderr, " %s\n", note);
 		}
 	}
 	fclose(fp);
@@ -622,9 +641,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		remote = remote_get(argv[0]);
 
 	transport = transport_get(remote, remote->url[0]);
-	if (verbose >= 2)
+	if (verbosity == VERBOSE)
 		transport->verbose = 1;
-	if (quiet)
+	if (verbosity == QUIET)
 		transport->verbose = -1;
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/builtin-merge.c b/builtin-merge.c
index 5e2b7f1..8259acb 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -50,6 +50,7 @@ static unsigned char head[20], stash[20];
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char *branch;
+static enum { QUIET, NORMAL, VERBOSE } verbosity = NORMAL;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -151,7 +152,27 @@ static int option_parse_n(const struct option *opt,
 	return 0;
 }
 
+static int option_parse_quiet(const struct option *opt,
+			  const char *arg, int unset)
+{
+	verbosity = QUIET;
+	return 0;
+}
+
+static int option_parse_verbose(const struct option *opt,
+			  const char *arg, int unset)
+{
+	verbosity = VERBOSE;
+	return 0;
+}
+
 static struct option builtin_merge_options[] = {
+	{ OPTION_CALLBACK, 'q', "quiet", NULL, NULL,
+		"operate quietly",
+		PARSE_OPT_NOARG, option_parse_quiet },
+	{ OPTION_CALLBACK, 'v', "verbose", NULL, NULL,
+		"be verbose",
+		PARSE_OPT_NOARG, option_parse_verbose },
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		"do not show a diffstat at the end of the merge",
 		PARSE_OPT_NOARG, option_parse_n },
@@ -249,7 +270,8 @@ static void restore_state(void)
 /* This is called when no merge was necessary. */
 static void finish_up_to_date(const char *msg)
 {
-	printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
+	if (verbosity > QUIET)
+		printf("%s%s\n", squash ? " (nothing to squash)" : "", msg);
 	drop_save();
 }
 
@@ -330,14 +352,15 @@ static void finish(const unsigned char *new_head, const char *msg)
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
 	else {
-		printf("%s\n", msg);
+		if (verbosity > QUIET)
+			printf("%s\n", msg);
 		strbuf_addf(&reflog_message, "%s: %s",
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
 		squash_message();
 	} else {
-		if (!merge_msg.len)
+		if (verbosity > QUIET && !merge_msg.len)
 			printf("No merge message -- not updating HEAD\n");
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
@@ -871,6 +894,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, builtin_merge_options,
 			builtin_merge_usage, 0);
+	if (verbosity > QUIET)
+		show_diffstat = 0;
 
 	if (squash) {
 		if (!allow_fast_forward)
@@ -1012,10 +1037,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		strcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));
 
-		printf("Updating %s..%s\n",
-			hex,
-			find_unique_abbrev(remoteheads->item->object.sha1,
-			DEFAULT_ABBREV));
+		if (verbosity > QUIET)
+			printf("Updating %s..%s\n",
+				hex,
+				find_unique_abbrev(remoteheads->item->object.sha1,
+				DEFAULT_ABBREV));
 		strbuf_addstr(&msg, "Fast forward");
 		if (have_message)
 			strbuf_addstr(&msg,
diff --git a/git-pull.sh b/git-pull.sh
index 75c3610..dc613db 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -16,6 +16,7 @@ cd_to_toplevel
 test -z "$(git ls-files -u)" ||
 	die "You are in the middle of a conflicted merge."
 
+verbosity=
 strategy_args= no_stat= no_commit= squash= no_ff= log_arg=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||")
@@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
 while :
 do
 	case "$1" in
+	-q|--quiet)
+		verbosity="$verbosity -q" ;;
+	-v|--verbose)
+		verbosity="$verbosity -v" ;;
 	-n|--no-stat|--no-summary)
 		no_stat=-n ;;
 	--stat|--summary)
@@ -121,7 +126,7 @@ test true = "$rebase" && {
 		"refs/remotes/$origin/$reflist" 2>/dev/null)"
 }
 orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
-git fetch --update-head-ok "$@" || exit 1
+git fetch $verbosity --update-head-ok "$@" || exit 1
 
 curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
 if test "$curr_head" != "$orig_head"
@@ -181,5 +186,6 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 test true = "$rebase" &&
 	exec git-rebase $strategy_args --onto $merge_head \
 	${oldremoteref:-$merge_head}
-exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
+exec git-merge $verbosity $no_stat $no_commit \
+	$squash $no_ff $log_arg $strategy_args \
 	"$merge_name" HEAD $merge_head
-- 
1.6.0.2.GIT

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

end of thread, other threads:[~2008-11-17 22:26 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-21 16:30 [PATCH] Teach/Fix pull/fetch -q/-v options Tuncer Ayaz
2008-10-27 10:08 ` Nanako Shiraishi
2008-10-28  3:21   ` Junio C Hamano
2008-11-01 17:23     ` Tuncer Ayaz
2008-11-07  3:26       ` Tuncer Ayaz
  -- strict thread matches above, loose matches on Subject: below --
2008-11-15 19:23 Tuncer Ayaz
2008-11-17 10:37 ` Tuncer Ayaz
2008-11-17 10:51   ` Junio C Hamano
2008-11-17 10:55     ` Tuncer Ayaz
2008-11-17 11:03     ` Constantine Plotnikov
2008-11-17 22:24       ` Tuncer Ayaz
2008-11-17 22:08     ` Tuncer Ayaz
2008-11-15  0:14 Tuncer Ayaz
2008-11-15  1:15 ` Junio C Hamano
2008-11-15  1:53   ` Tuncer Ayaz
2008-11-15  3:10     ` Junio C Hamano
2008-11-15 17:28       ` Tuncer Ayaz
2008-11-15 17:42   ` Tuncer Ayaz
2008-11-15 19:16     ` Tuncer Ayaz
2008-11-07  3:26 Tuncer Ayaz
2008-11-10 23:43 ` Tuncer Ayaz
2008-11-12 20:47   ` Junio C Hamano
2008-11-15  0:09     ` Tuncer Ayaz
2008-10-20 16:28 Tuncer Ayaz
2008-10-19 19:48 Tuncer Ayaz
2008-10-19 21:26 ` Junio C Hamano
2008-10-20 16:35   ` Tuncer Ayaz
2008-10-20 23:54     ` Junio C Hamano
2008-10-21 16:25       ` Tuncer Ayaz

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).