Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"
From: Alex Henrie <hidden>
Date: 2023-02-16 03:22:15
On Sun, Feb 5, 2023 at 9:41 AM Tao Klerks via GitGitGadget [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Tao Klerks <redacted> When "git pull" is called without a conflict-handling instruction or configuration, it displays a hint proposing "pull.rebase" and "pull.ff" config options for future handling. The hint offers three permanent settings, "merge", rebase", and "ff". The proposed command for "rebase" is "git config pull.rebase true". Unfortunately, this rebase configuration can easily lead to non-expert users accidentally rebasing not their own commits, instead others' commits, if the new commits they have locally before the "pull" include a merge of another branch, eg "main". Since 2018 in git version "2.18", it has supported a new rebase flag "--rebase-merges", with corresponding pull.rebase config option "merges". This new option is ideal for rebasing local work on "pull", as it will not "mangle"/flatten any local merge commits but rather recreate them. Change the pull conflict hint text to propose "pull.rebase merges" instead of "pull.rebase true", and "git pull --rebase=merges" instead of "git pull --rebase". Signed-off-by: Tao Klerks <redacted> --- pull: conflict hint pull.rebase suggestion should offer "merges" vs "true" Hint change as proposed in https://lore.kernel.org/git/xmqqa61uo3q0.fsf@gitster.g/ (local) Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1474%2FTaoK%2Ftao-fetch-rebase-hint-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1474/TaoK/tao-fetch-rebase-hint-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1474 builtin/pull.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)diff --git a/builtin/pull.c b/builtin/pull.c index 1ab4de0005d..535364fbb07 100644 --- a/builtin/pull.c +++ b/builtin/pull.c@@ -967,13 +967,13 @@ static void show_advice_pull_non_ff(void) "your next pull:\n" "\n" " git config pull.rebase false # merge\n" - " git config pull.rebase true # rebase\n" + " git config pull.rebase merges # rebase\n" " git config pull.ff only # fast-forward only\n" "\n" "You can replace \"git config\" with \"git config --global\" to set a default\n" - "preference for all repositories. You can also pass --rebase, --no-rebase,\n" - "or --ff-only on the command line to override the configured default per\n" - "invocation.\n")); + "preference for all repositories. You can also pass --rebase=merges,\n" + "--no-rebase, or --ff-only on the command line to override the configured\n" + "default per invocation.\n"));
Hi Tao, thank you for sharing your experiences with non-experts using `git pull`. I am always curious to see how people who are learning Git react to it, and I am very interested in making Git as straightforward as possible. I'm afraid I have several objections to this patch... - The proposed wording is likely to further confuse novices. It's asking the user to choose between the reconciliation strategies of merging and rebasing, but then says to use the unintuitive combination "rebase=merges" which sounds like it's going to make a merge commit at the end of the branch anyway. - The proposed wording makes it sound like there's something wrong with doing a regular rebase, but that's not usually the case because in practice a regular rebase is almost always equivalent to rebase=merges. A regular rebase may even be what the user really wants: For example, the user might choose to merge when pulling and then change their mind and decide that they really wanted to rebase. Repeating the pull with the regular -r or --rebase flag fixes the mistake. - `git pull -ri` (or its longer form `git pull --rebase=interactive`) is generally more useful than `git pull --rebase=merges`, but once rebase=merges has been specified, there's no way to specify rebase=interactive also. Recommending rebase=merges steers people away from rebase=interactive, hiding useful functionality from the user. Now, this is not to say that there's no room for improvement. I like the rebase=merges option and I wish everyone knew about it because there are situations where it really is the best option. I suggest leaving the existing text alone, but adding an additional paragraph, something like: Note that --rebase or pull.rebase=true will drop existing merge commits and rebase all of the commits from all of the merged branches. If you want to rebase but preserve existing merge commits, use --rebase=merges or pull.rebase=merges instead. -Alex