Re: [PATCH] advice: improve hint for diverging branches.

Thank you, all good points. I will work on the wording
improvements based on your suggestions.

"Consider the following options:" should be on a new line and
get a "hint:" prefix, I will fix that.


As far as the danger to the user, I was talking about the fact that
the user doing "git reset --hard" is going to lose any uncommitted
work as well as any commits currently in the branch.

 > "But more importantly, what guarantees your recomputation using
 > '*refspecs' here will match the result of the logic that computed
 > 'divergent', which certainly would have already known what commit we
 > tried to fast-forward our branch to, and where that commit came
 > from?  We shouldn't be computing the same thing twice, and in
 > different ways; that is a sure way to introduce inconsistent
 > results.

This makes sens, I did it this way because I wanted to get a remote and
branch name, not just hash id and it was difficult to get this information.
I will try to rework it so that it shares the code with the logic that 
determines
the divergence status.
Any tips on what's the best way to do that would be highly appreciated.

On 9/5/23 18:20, Junio C Hamano wrote:

"Konstantin Pereiaslov via GitGitGadget" [off-list ref]
writes:

quoted

From: Konstantin Pereiaslov <redacted>

Added a description of what the offered options will do and for pull
also offered a 3rd option during a pull - a hard reset.
This option should be helpful for the new users that accidentally
committed into the wrong branch which is a scenario I saw very
often.

cf. Documentation/SubmittingPatches:[[describe-changes]]

quoted

The resulting tooltip looks like this for pull:

hint: Diverging branches can't be fast-forwarded.
Consider the following options:

We do not give "hint:" prefix to this line???

quoted

hint:
hint: To merge remote changes into your branch:
hint:   git merge --no-ff
hint:
hint: To apply your changes on top of remote changes:
hint:   git rebase

Hmph, "apply" -> "replay" perhaps?

quoted

hint: To discard your local changes and apply the remote changes:

Here "apply" is definitely a misnomer.  Nothing is applied; you just
discard your work and adopt (or "accept") the state of the remote as
a whole.

quoted

hint:   git reset --hard refs/remotes/upstream/branch-name
hint:
hint: Disable this message with "git config advice.diverging false"

OK.

Overall, "... looks like this" should be shown a bit indented so
that the example stands out from the text that explains the example.

quoted

There is some danger because it's semi-destructive, but so are
other options offered if user doesn't know the commands to
revert back.

Sorry, but I do not quite understand what you want to say here.

quoted

@@ -1112,8 +1126,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
  
  	/* ff-only takes precedence over rebase */
  	if (opt_ff && !strcmp(opt_ff, "--ff-only")) {
-		if (divergent)
-			die_ff_impossible();
+		if (divergent) {
+			const char* pull_branch_spec = get_pull_branch(repo, *refspecs);

In this codebase, asterisk sticks to the variable/function
identifier, not types.

But more importantly, what guarantees your recomputation using
'*refspecs' here will match the result of the logic that computed
'divergent', which certainly would have already known what commit we
tried to fast-forward our branch to, and where that commit came
from?  We shouldn't be computing the same thing twice, and in
different ways; that is a sure way to introduce inconsistent
results.

quoted

+			die_ff_impossible_during_pull(pull_branch_spec);
+		}
  		opt_rebase = REBASE_FALSE;
  	}
  	/* If no action specified and we can't fast forward, then warn. */

base-commit: d814540bb75bbd2257f9a6bf59661a84fe8cf3cf

Thanks.