Hi Denton

On 08/01/2020 06:08, Denton Liu wrote:

Hi Phillip,

Sorry for the late reply. I'm back in school so I've been pretty busy
lately.

On Tue, Dec 31, 2019 at 10:34:30AM +0000, Phillip Wood wrote:

quoted

For `merge --autostash` I think we need to consider the interaction of
`--no-commit` with `--autostash` as `stash pop` will refuse to run if the
merge modified any of the stashed paths.

The only time when we run the `stash pop` is when we either commit the
merge or abort it. With this in mind, what do you think of the following
test cases? If they look good, I can squash them into the appropriate
patch and send in a reroll.

Ah I misunderstood what happened with --autostash and --no-commit, the 
tests basically look fine (I've got one comment below).

One other thing - if the user runs `git reset` or `git checkout 
<branch>` then cleanup_branch_state() removes MERGE_HEAD, MERGE_MSG etc. 
If we're not already doing so then I think we should remove 
MERGE_AUTOSTASH as well or you can get into a situation where the user 
does something like

   git merge --autostash <something> # results in conflicts
   git reset --hard <somewhere else>
   git merge <something> # succeeds and confusingly pops previous stash

Running `git reset` doesn't make sense unless they want to discard the 
stashed changes as well. This is a difference with rebase where you 
cannot lose the stashed changes by running `git reset`, the only way 
they can get lost is by running `rebase --quit`.  We could always add a 
warning about it throwing away the stashed changes in the future.

I still not keen on the name `--autostash` as it's not automatic. 
`--stash` would make more sense to me. We'd have to deprecate `rebase 
--autostash` in favor of `rebase --stash` to change it but it if we want 
to change the name it would be better now before adding `--autostash` to 
merge and pull - what do you think?

	diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
	index e0c8a0bad4..af5db58e16 100755
	--- a/t/t7600-merge.sh
	+++ b/t/t7600-merge.sh
	@@ -727,6 +727,33 @@ test_expect_success 'conflicted merge with --autostash, --abort restores stash'
		test_cmp file.1 file
	 '
	
	+test_expect_success 'completed merge with --no-commit and --autostash' '
	+	git reset --hard c1 &&
	+	git merge-file file file.orig file.9 &&

Is this a complicated way of getting some unstaged changes so we can 
stash them or have I missed something?

Best Wishes

Phillip

	+	git diff >expect &&
	+	git merge --no-commit --autostash c2 &&
	+	git stash show -p MERGE_AUTOSTASH >actual &&
	+	test_cmp expect actual &&
	+	git commit 2>err &&
	+	test_i18ngrep "Applied autostash." err &&
	+	git show HEAD:file >merge-result &&
	+	test_cmp result.1-5 merge-result &&
	+	test_cmp result.1-5-9 file
	+'
	+
	+test_expect_success 'aborted merge with --no-commit and --autostash' '
	+	git reset --hard c1 &&
	+	git merge-file file file.orig file.9 &&
	+	git diff >expect &&
	+	git merge --no-commit --autostash c2 &&
	+	git stash show -p MERGE_AUTOSTASH >actual &&
	+	test_cmp expect actual &&
	+	git merge --abort 2>err &&
	+	test_i18ngrep "Applied autostash." err &&
	+	git diff >actual &&
	+	test_cmp expect actual
	+'
	+
	 test_expect_success 'merge with conflicted --autostash changes' '
		git reset --hard c1 &&
		git merge-file file file.orig file.9y &&

Thanks,

Denton

quoted

Best Wishes

Phillip