[PATCH] remote: qualify "git pull" advice for non-upstream branches

[PATCH] remote: qualify "git pull" advice for non-upstream branches

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When "git status" reports the local branch is behind or has diverged
from the push branch, the advice suggested a bare "git pull". That
follows the upstream, which may live on a different remote, so emit
"git pull <remote> <branch>" instead.

Also enable the pull and divergence advice for push-branch
comparisons; they were previously only set for the upstream.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    remote: qualify "git pull" advice for non-upstream branches

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2301%2FHaraldNordgren%2Fstatus-pull-advice-qualified-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2301/HaraldNordgren/status-pull-advice-qualified-v1
Pull-Request: https://github.com/git/git/pull/2301

 remote.c                 | 47 +++++++++++++++++++++++++++++++---------
 t/t6040-tracking-info.sh | 41 +++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/remote.c b/remote.c
index a664cd166a..d1e09079cb 100644
--- a/remote.c
+++ b/remote.c
@@ -2267,6 +2267,8 @@ static void format_branch_comparison(struct strbuf *sb,
 				     bool up_to_date,
 				     int ours, int theirs,
 				     const char *branch_name,
+				     const char *push_remote_name,
+				     const char *push_branch_name,
 				     enum ahead_behind_flags abf,
 				     unsigned flags)
 {
@@ -2302,9 +2304,15 @@ static void format_branch_comparison(struct strbuf *sb,
 			       "and can be fast-forwarded.\n",
 			   theirs),
 			branch_name, theirs);
-		if (use_pull_advice && advice_enabled(ADVICE_STATUS_HINTS))
-			strbuf_addstr(sb,
-				_("  (use \"git pull\" to update your local branch)\n"));
+		if (use_pull_advice && advice_enabled(ADVICE_STATUS_HINTS)) {
+			if (push_remote_name && push_branch_name)
+				strbuf_addf(sb,
+					_("  (use \"git pull %s %s\" to update your local branch)\n"),
+					push_remote_name, push_branch_name);
+			else
+				strbuf_addstr(sb,
+					_("  (use \"git pull\" to update your local branch)\n"));
+		}
 	} else {
 		strbuf_addf(sb,
 			Q_("Your branch and '%s' have diverged,\n"
@@ -2315,9 +2323,15 @@ static void format_branch_comparison(struct strbuf *sb,
 			       "respectively.\n",
 			   ours + theirs),
 			branch_name, ours, theirs);
-		if (use_divergence_advice && advice_enabled(ADVICE_STATUS_HINTS))
-			strbuf_addstr(sb,
-				_("  (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
+		if (use_divergence_advice && advice_enabled(ADVICE_STATUS_HINTS)) {
+			if (push_remote_name && push_branch_name)
+				strbuf_addf(sb,
+					_("  (use \"git pull %s %s\" if you want to integrate the remote branch with yours)\n"),
+					push_remote_name, push_branch_name);
+			else
+				strbuf_addstr(sb,
+					_("  (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
+		}
 	}
 }
 
@@ -2355,6 +2369,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 		int ours, theirs, cmp;
 		int is_upstream, is_push;
 		unsigned flags = 0;
+		const char *push_remote_name = NULL;
+		const char *push_branch_name = NULL;
 
 		full_ref = resolve_compare_branch(branch,
 						  branches.items[i].string);
@@ -2396,13 +2412,24 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 		if (reported)
 			strbuf_addstr(sb, "\n");
 
-		if (is_upstream)
+		if (is_upstream || is_push) {
 			flags |= ENABLE_ADVICE_PULL;
-		if (is_push)
+			if (show_divergence_advice)
+				flags |= ENABLE_ADVICE_DIVERGENCE;
+		}
+		if (is_push) {
 			flags |= ENABLE_ADVICE_PUSH;
-		if (show_divergence_advice && is_upstream)
-			flags |= ENABLE_ADVICE_DIVERGENCE;
+			push_remote_name = pushremote_for_branch(branch, NULL);
+			if (push_remote_name &&
+			    skip_prefix(full_ref, "refs/remotes/", &push_branch_name) &&
+			    skip_prefix(push_branch_name, push_remote_name, &push_branch_name) &&
+			    *push_branch_name == '/')
+				push_branch_name++;
+			else
+				push_remote_name = NULL;
+		}
 		format_branch_comparison(sb, !cmp, ours, theirs, short_ref,
+					 push_remote_name, push_branch_name,
 					 abf, flags);
 		reported = 1;
 
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 0242b5bf7a..3199063762 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -529,6 +529,7 @@ test_expect_success 'status.compareBranches with diverged push branch' '
 
 	Your branch and ${SQ}origin/feature8${SQ} have diverged,
 	and have 1 and 1 different commits each, respectively.
+	  (use "git pull origin feature8" if you want to integrate the remote branch with yours)
 
 	nothing to commit, working tree clean
 	EOF
@@ -646,4 +647,44 @@ test_expect_success 'status.compareBranches with remapped push and upstream remo
 	test_cmp expect actual
 '
 
+test_expect_success 'status.compareBranches with behind push branch suggests qualified pull' '
+	test_config -C test push.default current &&
+	test_config -C test remote.pushDefault origin &&
+	test_config -C test status.compareBranches "@{upstream} @{push}" &&
+	git -C test checkout -b feature13 upstream/main &&
+	(cd test && advance work13) &&
+	git -C test push origin &&
+	git -C test reset --hard HEAD^ &&
+	git -C test status >actual &&
+	cat >expect <<-EOF &&
+	On branch feature13
+	Your branch is up to date with ${SQ}upstream/main${SQ}.
+
+	Your branch is behind ${SQ}origin/feature13${SQ} by 1 commit, and can be fast-forwarded.
+	  (use "git pull origin feature13" to update your local branch)
+
+	nothing to commit, working tree clean
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'status.compareBranches with remapped push and behind push branch' '
+	test_config -C test remote.pushDefault origin &&
+	test_config -C test remote.origin.push refs/heads/feature14:refs/heads/remapped14 &&
+	test_config -C test status.compareBranches "@{push}" &&
+	git -C test checkout -b feature14 upstream/main &&
+	(cd test && advance work14) &&
+	git -C test push &&
+	git -C test reset --hard HEAD^ &&
+	git -C test status >actual &&
+	cat >expect <<-EOF &&
+	On branch feature14
+	Your branch is behind ${SQ}origin/remapped14${SQ} by 1 commit, and can be fast-forwarded.
+	  (use "git pull origin remapped14" to update your local branch)
+
+	nothing to commit, working tree clean
+	EOF
+	test_cmp expect actual
+'
+
 test_done

base-commit: 29bd7ed5127255713c1ac2f43b7c6f257d7b4594
-- 
gitgitgadget

[PATCH v2] remote: qualify "git pull" advice for non-upstream branches

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When "git status" reports the local branch is behind the push
branch, the advice suggested a bare "git pull". That follows the
upstream, which may live on a different remote, so emit
"git pull <remote> <branch>" instead.

Also enable the pull advice for push-branch comparisons; it was
previously only set for the upstream.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    remote: qualify "git pull" advice for non-upstream branches
    
    Remove the message only when diverged from push-branch, because it was
    in the way when I used it now on my real branch.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2301%2FHaraldNordgren%2Fstatus-pull-advice-qualified-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2301/HaraldNordgren/status-pull-advice-qualified-v2
Pull-Request: https://github.com/git/git/pull/2301

Range-diff vs v1:

 1:  0a06883cc8 ! 1:  1f06873f82 remote: qualify "git pull" advice for non-upstream branches
     @@ Metadata
       ## Commit message ##
          remote: qualify "git pull" advice for non-upstream branches
      
     -    When "git status" reports the local branch is behind or has diverged
     -    from the push branch, the advice suggested a bare "git pull". That
     -    follows the upstream, which may live on a different remote, so emit
     +    When "git status" reports the local branch is behind the push
     +    branch, the advice suggested a bare "git pull". That follows the
     +    upstream, which may live on a different remote, so emit
          "git pull <remote> <branch>" instead.
      
     -    Also enable the pull and divergence advice for push-branch
     -    comparisons; they were previously only set for the upstream.
     +    Also enable the pull advice for push-branch comparisons; it was
     +    previously only set for the upstream.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ remote.c: int format_tracking_info(struct branch *branch, struct strbuf *sb,
       			strbuf_addstr(sb, "\n");
       
      -		if (is_upstream)
     -+		if (is_upstream || is_push) {
     ++		if (is_upstream || is_push)
       			flags |= ENABLE_ADVICE_PULL;
      -		if (is_push)
     -+			if (show_divergence_advice)
     -+				flags |= ENABLE_ADVICE_DIVERGENCE;
     -+		}
     +-			flags |= ENABLE_ADVICE_PUSH;
     + 		if (show_divergence_advice && is_upstream)
     + 			flags |= ENABLE_ADVICE_DIVERGENCE;
      +		if (is_push) {
     - 			flags |= ENABLE_ADVICE_PUSH;
     --		if (show_divergence_advice && is_upstream)
     --			flags |= ENABLE_ADVICE_DIVERGENCE;
     ++			flags |= ENABLE_ADVICE_PUSH;
      +			push_remote_name = pushremote_for_branch(branch, NULL);
      +			if (push_remote_name &&
      +			    skip_prefix(full_ref, "refs/remotes/", &push_branch_name) &&
     @@ remote.c: int format_tracking_info(struct branch *branch, struct strbuf *sb,
       
      
       ## t/t6040-tracking-info.sh ##
     -@@ t/t6040-tracking-info.sh: test_expect_success 'status.compareBranches with diverged push branch' '
     - 
     - 	Your branch and ${SQ}origin/feature8${SQ} have diverged,
     - 	and have 1 and 1 different commits each, respectively.
     -+	  (use "git pull origin feature8" if you want to integrate the remote branch with yours)
     - 
     - 	nothing to commit, working tree clean
     - 	EOF
      @@ t/t6040-tracking-info.sh: test_expect_success 'status.compareBranches with remapped push and upstream remo
       	test_cmp expect actual
       '


 remote.c                 | 44 ++++++++++++++++++++++++++++++++--------
 t/t6040-tracking-info.sh | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/remote.c b/remote.c
index a664cd166a..e096fdb674 100644
--- a/remote.c
+++ b/remote.c
@@ -2267,6 +2267,8 @@ static void format_branch_comparison(struct strbuf *sb,
 				     bool up_to_date,
 				     int ours, int theirs,
 				     const char *branch_name,
+				     const char *push_remote_name,
+				     const char *push_branch_name,
 				     enum ahead_behind_flags abf,
 				     unsigned flags)
 {
@@ -2302,9 +2304,15 @@ static void format_branch_comparison(struct strbuf *sb,
 			       "and can be fast-forwarded.\n",
 			   theirs),
 			branch_name, theirs);
-		if (use_pull_advice && advice_enabled(ADVICE_STATUS_HINTS))
-			strbuf_addstr(sb,
-				_("  (use \"git pull\" to update your local branch)\n"));
+		if (use_pull_advice && advice_enabled(ADVICE_STATUS_HINTS)) {
+			if (push_remote_name && push_branch_name)
+				strbuf_addf(sb,
+					_("  (use \"git pull %s %s\" to update your local branch)\n"),
+					push_remote_name, push_branch_name);
+			else
+				strbuf_addstr(sb,
+					_("  (use \"git pull\" to update your local branch)\n"));
+		}
 	} else {
 		strbuf_addf(sb,
 			Q_("Your branch and '%s' have diverged,\n"
@@ -2315,9 +2323,15 @@ static void format_branch_comparison(struct strbuf *sb,
 			       "respectively.\n",
 			   ours + theirs),
 			branch_name, ours, theirs);
-		if (use_divergence_advice && advice_enabled(ADVICE_STATUS_HINTS))
-			strbuf_addstr(sb,
-				_("  (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
+		if (use_divergence_advice && advice_enabled(ADVICE_STATUS_HINTS)) {
+			if (push_remote_name && push_branch_name)
+				strbuf_addf(sb,
+					_("  (use \"git pull %s %s\" if you want to integrate the remote branch with yours)\n"),
+					push_remote_name, push_branch_name);
+			else
+				strbuf_addstr(sb,
+					_("  (use \"git pull\" if you want to integrate the remote branch with yours)\n"));
+		}
 	}
 }
 
@@ -2355,6 +2369,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 		int ours, theirs, cmp;
 		int is_upstream, is_push;
 		unsigned flags = 0;
+		const char *push_remote_name = NULL;
+		const char *push_branch_name = NULL;
 
 		full_ref = resolve_compare_branch(branch,
 						  branches.items[i].string);
@@ -2396,13 +2412,23 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 		if (reported)
 			strbuf_addstr(sb, "\n");
 
-		if (is_upstream)
+		if (is_upstream || is_push)
 			flags |= ENABLE_ADVICE_PULL;
-		if (is_push)
-			flags |= ENABLE_ADVICE_PUSH;
 		if (show_divergence_advice && is_upstream)
 			flags |= ENABLE_ADVICE_DIVERGENCE;
+		if (is_push) {
+			flags |= ENABLE_ADVICE_PUSH;
+			push_remote_name = pushremote_for_branch(branch, NULL);
+			if (push_remote_name &&
+			    skip_prefix(full_ref, "refs/remotes/", &push_branch_name) &&
+			    skip_prefix(push_branch_name, push_remote_name, &push_branch_name) &&
+			    *push_branch_name == '/')
+				push_branch_name++;
+			else
+				push_remote_name = NULL;
+		}
 		format_branch_comparison(sb, !cmp, ours, theirs, short_ref,
+					 push_remote_name, push_branch_name,
 					 abf, flags);
 		reported = 1;
 
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 0242b5bf7a..b686bf356a 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -646,4 +646,44 @@ test_expect_success 'status.compareBranches with remapped push and upstream remo
 	test_cmp expect actual
 '
 
+test_expect_success 'status.compareBranches with behind push branch suggests qualified pull' '
+	test_config -C test push.default current &&
+	test_config -C test remote.pushDefault origin &&
+	test_config -C test status.compareBranches "@{upstream} @{push}" &&
+	git -C test checkout -b feature13 upstream/main &&
+	(cd test && advance work13) &&
+	git -C test push origin &&
+	git -C test reset --hard HEAD^ &&
+	git -C test status >actual &&
+	cat >expect <<-EOF &&
+	On branch feature13
+	Your branch is up to date with ${SQ}upstream/main${SQ}.
+
+	Your branch is behind ${SQ}origin/feature13${SQ} by 1 commit, and can be fast-forwarded.
+	  (use "git pull origin feature13" to update your local branch)
+
+	nothing to commit, working tree clean
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'status.compareBranches with remapped push and behind push branch' '
+	test_config -C test remote.pushDefault origin &&
+	test_config -C test remote.origin.push refs/heads/feature14:refs/heads/remapped14 &&
+	test_config -C test status.compareBranches "@{push}" &&
+	git -C test checkout -b feature14 upstream/main &&
+	(cd test && advance work14) &&
+	git -C test push &&
+	git -C test reset --hard HEAD^ &&
+	git -C test status >actual &&
+	cat >expect <<-EOF &&
+	On branch feature14
+	Your branch is behind ${SQ}origin/remapped14${SQ} by 1 commit, and can be fast-forwarded.
+	  (use "git pull origin remapped14" to update your local branch)
+
+	nothing to commit, working tree clean
+	EOF
+	test_cmp expect actual
+'
+
 test_done

base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
-- 
gitgitgadget

Re: [PATCH v2] remote: qualify "git pull" advice for non-upstream branches

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> When "git status" reports the local branch is behind the push
> branch, the advice suggested a bare "git pull". That follows the
> upstream, which may live on a different remote, so emit
> "git pull <remote> <branch>" instead.

Hmph, shouldn't this be done conditionally, though?  Most new users
follow the recommended pattern to set branch.<name>.merge so that
"git pull" would do the right thing for them, I presume, even when
they are using triangular workflow to push to a different remote
than the remote they pull from, so the new and more verbose message
would not help the users any more than the existing message, right?

Can the code tell the situation where the extra part of the message
would help and give it only then?