Re: [PATCH v1 0/2] Be nicer to the user on tracked/untracked merge conflicts

Jonathan <git.jonathan.bressat@gmail.com> writes:

Correct. If you want to do this correctly, you'd need to make sure that you'd clobber untracked files ONLY when you are not losing any information.

And even with that, I think some existing users will be hurt with this change in a huge way. They may have untracked change locally because they are not quite done with it yet, and somebody else throws a pull request at them that has the same change as the local modification.

They make a trial merge, look at the result, and discard it because there are also unwanted changes in the branch they pulled into.

    $ git pull $URL $branch ;# responding to the pull request
    ... examine the result, finding it unsatisfactory ...
    $ git reset --hard ORIG_HEAD
    ... now we are back to where we started; well not really ...

Now, without this change, "git pull" used to stop until they stashed away the untracked change safely. But with this change, "git pull" will succeed, and then "reset --hard" will discard it together with other changes that came to us from $URL/$branch. They lost their local, uncommitted change.

And "but you can pull the equivalent out of $URL/$branch" is not a good excuse. They may not notice the lossage long after having dealt with this pull request (there are busy people who are handling many pull requests from many people) and they have been relying on "git pull" that never clobbers their local uncommitted changes. And when they noticed the lossage, they may not even remember which one of pull requests happened to have an identical change as their local change to cause this lossage, simply because "git pull" that used to stop just continued without a noise.

So, I am not sure if this is really a good idea to begin with. It certainly would make it slightly simpler in a trivial case, but it surely looks like a dangerous behaviour change, especially if it is done unconditionally.

Wait. Isn't it backwards? The existing tests _may_ be casting an undesirable current behaviour in stone, but most of the time it is protecting existing user's expectations. If you have an untracked file, you can rest assured that they won't be clobbered by a merge.

So we'd need to think twice and carefully examine if it makes sense to update the expectations. I haven't read the change to the tests, so I cannot tell which case it is.

Thanks.