[PATCH] status: respect advice.statusHints for ahead/behind advice

[PATCH] status: respect advice.statusHints for ahead/behind advice

[Jeff King]

If the user has unset advice.statusHints, we already
suppress the "use git reset to..." hints in each stanza. The
new "use git push to publish..." hint is the same type of
hint. Let's respect statusHints for it, rather than making
the user set yet another advice flag.

Signed-off-by: Jeff King <peff@peff.net>
---
On top of mm/status-push-pull-advise.

I left "git checkout" alone, though I'd also like to turn it off there,
too. Should it get a separate advice option there? Should it simply
respect statusHints (it seems odd because I know that "status" there
means "git status", not "hints about the status of your repo")?

 builtin/checkout.c |  2 +-
 remote.c           | 17 ++++++++++-------
 remote.h           |  2 +-
 wt-status.c        |  2 +-
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 781295b..28146d1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -544,7 +544,7 @@ static void report_tracking(struct branch_info *new)
 	struct strbuf sb = STRBUF_INIT;
 	struct branch *branch = branch_get(new->name);
 
-	if (!format_tracking_info(branch, &sb))
+	if (!format_tracking_info(branch, &sb, 1))
 		return;
 	fputs(sb.buf, stdout);
 	strbuf_release(&sb);
diff --git a/remote.c b/remote.c
index 9c19689..176a777 100644
--- a/remote.c
+++ b/remote.c
@@ -1617,7 +1617,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
 /*
  * Return true when there is anything to report, otherwise false.
  */
-int format_tracking_info(struct branch *branch, struct strbuf *sb)
+int format_tracking_info(struct branch *branch, struct strbuf *sb, int advice)
 {
 	int num_ours, num_theirs;
 	const char *base;
@@ -1633,8 +1633,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			   "Your branch is ahead of '%s' by %d commits.\n",
 			   num_ours),
 			base, num_ours);
-		strbuf_addf(sb,
-			_("  (use \"git push\" to publish your local commits)\n"));
+		if (advice)
+			strbuf_addf(sb,
+				_("  (use \"git push\" to publish your local commits)\n"));
 	} else if (!num_ours) {
 		strbuf_addf(sb,
 			Q_("Your branch is behind '%s' by %d commit, "
@@ -1643,8 +1644,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			       "and can be fast-forwarded.\n",
 			   num_theirs),
 			base, num_theirs);
-		strbuf_addf(sb,
-			_("  (use \"git pull\" to update your local branch)\n"));
+		if (advice)
+			strbuf_addf(sb,
+				_("  (use \"git pull\" to update your local branch)\n"));
 	} else {
 		strbuf_addf(sb,
 			Q_("Your branch and '%s' have diverged,\n"
@@ -1655,8 +1657,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			       "respectively.\n",
 			   num_theirs),
 			base, num_ours, num_theirs);
-		strbuf_addf(sb,
-			_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+		if (advice)
+			strbuf_addf(sb,
+				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
 	return 1;
 }
diff --git a/remote.h b/remote.h
index 251d8fd..ac504a5 100644
--- a/remote.h
+++ b/remote.h
@@ -153,7 +153,7 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 
 /* Reporting of tracking info */
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
-int format_tracking_info(struct branch *branch, struct strbuf *sb);
+int format_tracking_info(struct branch *branch, struct strbuf *sb, int advice);
 
 struct ref *get_local_heads(void);
 /*
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..b48c8cf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -755,7 +755,7 @@ static void wt_status_print_tracking(struct wt_status *s)
 	if (prefixcmp(s->branch, "refs/heads/"))
 		return;
 	branch = branch_get(s->branch + 11);
-	if (!format_tracking_info(branch, &sb))
+	if (!format_tracking_info(branch, &sb, advice_status_hints))
 		return;
 
 	for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1)
-- 
1.7.12.4.42.ge2b5b43

Re: [PATCH] status: respect advice.statusHints for ahead/behind advice

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> If the user has unset advice.statusHints, we already
> suppress the "use git reset to..." hints in each stanza. The
> new "use git push to publish..." hint is the same type of
> hint. Let's respect statusHints for it, rather than making
> the user set yet another advice flag.

Sorry, I should have made this in the patch.

With or without my suggestion below,

Acknowledged-by: Matthieu Moy <Matthieu.Moy@imag.fr>

> I left "git checkout" alone, though I'd also like to turn it off there,
> too. Should it get a separate advice option there? Should it simply
> respect statusHints (it seems odd because I know that "status" there
> means "git status", not "hints about the status of your repo")?

I'd respect the same statusHint indeed. It's the same message, so I
think the same category of users will want to disable it. The name
"statusHint" may not be the ideal one, but if you see the messages in
"git checkout" as an excerpt of "git status", it actually makes sense.

As a nice side effect, it also makes the code/patch simpler (no need to
add the "advice" argument).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] status: respect advice.statusHints for ahead/behind advice

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> If the user has unset advice.statusHints, we already
> suppress the "use git reset to..." hints in each stanza. The
> new "use git push to publish..." hint is the same type of
> hint. Let's respect statusHints for it, rather than making
> the user set yet another advice flag.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> On top of mm/status-push-pull-advise.
>
> I left "git checkout" alone, though I'd also like to turn it off there,
> too. Should it get a separate advice option there? Should it simply
> respect statusHints (it seems odd because I know that "status" there
> means "git status", not "hints about the status of your repo")?

I agree that we should have a toggle to turn it off and I think it
is OK to reuse the same "hints about the status" option for this
purpose.  It is not like there is a released version that already
gives the advice (possibly with some other option to turn it off)
and you are changing the behaviour of "checkout" by suddenly making
it honor statusHints advice.

So let's do a lot simpler patch instead.

-- >8 --
From: Jeff King <peff@peff.net>
Date: Mon, 3 Dec 2012 01:16:57 -0500

If the user has unset advice.statusHints, we already
suppress the "use git reset to..." hints in each stanza. The
new "use git push to publish..." hint is the same type of
hint. Let's respect statusHints for it, rather than making
the user set yet another advice flag.

Signed-off-by: Jeff King <peff@peff.net>
Acked-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 remote.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/remote.c b/remote.c
index 9c19689..18dc8ec 100644
--- a/remote.c
+++ b/remote.c
@@ -1633,8 +1633,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			   "Your branch is ahead of '%s' by %d commits.\n",
 			   num_ours),
 			base, num_ours);
-		strbuf_addf(sb,
-			_("  (use \"git push\" to publish your local commits)\n"));
+		if (advice_status_hints)
+			strbuf_addf(sb,
+				_("  (use \"git push\" to publish your local commits)\n"));
 	} else if (!num_ours) {
 		strbuf_addf(sb,
 			Q_("Your branch is behind '%s' by %d commit, "
@@ -1643,8 +1644,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			       "and can be fast-forwarded.\n",
 			   num_theirs),
 			base, num_theirs);
-		strbuf_addf(sb,
-			_("  (use \"git pull\" to update your local branch)\n"));
+		if (advice_status_hints)
+			strbuf_addf(sb,
+				_("  (use \"git pull\" to update your local branch)\n"));
 	} else {
 		strbuf_addf(sb,
 			Q_("Your branch and '%s' have diverged,\n"
@@ -1655,8 +1657,9 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			       "respectively.\n",
 			   num_theirs),
 			base, num_ours, num_theirs);
-		strbuf_addf(sb,
-			_("  (use \"git pull\" to merge the remote branch into yours)\n"));
+		if (advice_status_hints)
+			strbuf_addf(sb,
+				_("  (use \"git pull\" to merge the remote branch into yours)\n"));
 	}
 	return 1;
 }
-- 
1.8.0.1.420.g52a5207

Re: [PATCH] status: respect advice.statusHints for ahead/behind advice

[Jeff King]

On Mon, Dec 03, 2012 at 09:11:32AM -0800, Junio C Hamano wrote:

> > On top of mm/status-push-pull-advise.
> >
> > I left "git checkout" alone, though I'd also like to turn it off there,
> > too. Should it get a separate advice option there? Should it simply
> > respect statusHints (it seems odd because I know that "status" there
> > means "git status", not "hints about the status of your repo")?
> 
> I agree that we should have a toggle to turn it off and I think it
> is OK to reuse the same "hints about the status" option for this
> purpose.  It is not like there is a released version that already
> gives the advice (possibly with some other option to turn it off)
> and you are changing the behaviour of "checkout" by suddenly making
> it honor statusHints advice.
> 
> So let's do a lot simpler patch instead.
> 
> -- >8 --

Perfect, thanks.

-Peff

Re: [PATCH] status: respect advice.statusHints for ahead/behind advice

[Matthieu Moy]

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

> So let's do a lot simpler patch instead.
>
> -- >8 --
> From: Jeff King <peff@peff.net>
> Date: Mon, 3 Dec 2012 01:16:57 -0500
>
> If the user has unset advice.statusHints, we already
> suppress the "use git reset to..." hints in each stanza. The
> new "use git push to publish..." hint is the same type of
> hint. Let's respect statusHints for it, rather than making
> the user set yet another advice flag.

You may want to squash this on top:

-- >8 --

>From 87a438bd06dd689bd37949e762690ab5dbfc5ff8 Mon Sep 17 00:00:00 2001
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Tue, 4 Dec 2012 10:15:03 +0100
Subject: [PATCH] document that statusHints affects git checkout

---
 Documentation/config.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e70216d..bf8f911 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -160,9 +160,10 @@ advice.*::
                it resulted in a non-fast-forward error.
        statusHints::
                Show directions on how to proceed from the current
-               state in the output of linkgit:git-status[1] and in
+               state in the output of linkgit:git-status[1], in
                the template shown when writing commit messages in
-               linkgit:git-commit[1].
+               linkgit:git-commit[1], and in the help message shown
+               by linkgit:git-checkout[1] when switching branch.
        commitBeforeMerge::
                Advice shown when linkgit:git-merge[1] refuses to
                merge to avoid overwriting local changes.
-- 
1.8.0.319.g8abfee4



-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/