bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

Hi Junio,

Sorry for the late reply. Too bad neither of your emails were recorded 
in the bug report thread 
(https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20292).

Maybe it would be better if you file a separate bug report.

On 27.10.2016 20:20, Junio C Hamano wrote:

> The way Git is designed to be used is for the user to edit the
> latter to come up with a conflict resolution in the working tree,
> then add that result to the index, after the user is satisfied with
> it, before continuing (typically, but not necessarily, to record the
> result with "git commit").
>
> Now, what about the cleanly automerged paths?  They are added to the
> index and this is by design.  You can see why this is necessary by
> doing:

Thank you for the explanation. It does make sense now, but that does not 
necessarily mean that we should change what Emacs does about the conflicts.

You see, the change in question is in vc-git, which is part of our VC 
package, which does not support every nifty feature of each VCS it 
handles, and instead tries to provide a unified UI to the whole bunch of 
them. In a lot of cases that means that we exchange power for simplicity.

And while we might be happier with a more full-featured solution for 
dealing with conflicts (a new, better visualization, more controls), we 
don't have that now, and vc-git-resolve-conflicts is the faster, simple 
alternative we have come up with for now.

>  - But before doing "git add" to tell Git that you are done with
>    these paths, run "git diff" (no other parameters).  You may have
>    to (setq vc-git-resolve-conflicts nil) for that
>
> You will notice that this invocation of "git diff" show concisely
> how the result of your conflict resolution differs from either of
> the branch.  This is used by Git users as one of the final step of
> verification before telling Git "I now am done with the conflict in
> this path and resolution is good" by issuing "git add" for the path.
>
> You will also notice that this invocation of "git diff" does not
> clutter with changes that have been auto-merged cleanly.  At this
> step of the workflow to resolve conflicts, the user is concentrating
> on the paths that had conflicts, and it is crucial that cleanly
> auto-merged paths do not get in the way.  The user can still view
> the overall picture by asking "git diff HEAD" (to see both
> automerged result and hand-resolved result, the latter of which may
> or may not have been added to the index) and "git diff --cached" (to
> see automerged result and hand-resolved and then "git add"'ed
> result).
>
> So, there is no "Bad news, everybody!" in the behaviour you
> observed.

You have quoted a fairly early message in that bug discussion. Later on, 
we have reached a solution that seems to have satisfied all 
participants. Although it still does the automatic 'git add' call which 
you seems to consider the main problem.

>> What shall we do? Unstage the automatically-staged files? Revert the
>> changes from this bug? It seems Git really wants the changes staged
>> after the conflict resolution.
>
> I do not know what you thought you can achieve by "unstage the
> auto-merged paths?" here, but perhaps you were expecting "git diff"
> (no arguments) would be the way to view all the changes that a mergy
> operation with conflicted states brought in?

Not really. We get the "unstage the automatically-staged files" effect 
by calling "git reset" when the user has made an explicit configuration 
to do that and there's no ".git/MERGE_HEAD" (see the definition of 
vc-git-resolve-when-done).

To put it simply, we wanted to be able to easily resolve the merge 
conflict after e.g. 'git stash pop' without having to additionally 
interact with Git outside of Emacs. The current behavior is a compromise 
that allows us to achieve that. It also mirrors a similar feature in our 
Bazaar VC backend (which certain developers have grown accustomed to).

> If that (i.e. "view
> all the changes") was what you wanted to achieve, then neither
> "unstage the auto-merged paths" nor "automatically 'git add' upon
> saving the buffer" is a good solution.
>
> I was told by somebody that the message I am responding to
> eventually resulted in vc-git-resolve-conflicts that defaults to
> true in Emacs 25.1, which lead to "automatically 'git add' upon
> saving the buffer".  I think this variable and its default is a big
> disservice to Git users' whose daily work involves lots of conflict
> resolutions in mergy operations.

The ability to 'git diff' which you have described seems fairly obscure 
to me and the majority of Emacs/Git users, so even if we make 
vc-git-resolve-conflicts default to nil, it's unlikely that the majority 
of users will really benefit from that.

You, as a core Git developer, represent the informed minority, and as 
such, it's probably not too much to expect that you and the others can 
customize vc-git-resolve-conflicts to nil without much trouble.

That said, I was not the one to add this variable, and though so far I 
have found it useful enough, I wouldn't miss it too much either.

On the other hand, we haven't received any bug reports about it so far, 
so maybe most users either don't notice the new behavior, or have found 
it handy.

Best regards,
Dmitry.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 16 Nov 2016 02:25:32 +0200
> 
> Sorry for the late reply. Too bad neither of your emails were recorded 
> in the bug report thread 
> (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20292).

I don't even see the message you are responding to.  Was it sent only
to you, Dmitry?

> The ability to 'git diff' which you have described seems fairly obscure 
> to me and the majority of Emacs/Git users, so even if we make 
> vc-git-resolve-conflicts default to nil, it's unlikely that the majority 
> of users will really benefit from that.
> 
> You, as a core Git developer, represent the informed minority, and as 
> such, it's probably not too much to expect that you and the others can 
> customize vc-git-resolve-conflicts to nil without much trouble.

Perhaps there could be a way to customize vc-git so that it caters to
such advanced users?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

On 16.11.2016 19:30, Eli Zaretskii wrote:

> I don't even see the message you are responding to.  Was it sent only
> to you, Dmitry?

It was addressed to both me and the bug email, but didn't arrive at the 
latter. I'm attaching the full message here.

>> You, as a core Git developer, represent the informed minority, and as
>> such, it's probably not too much to expect that you and the others can
>> customize vc-git-resolve-conflicts to nil without much trouble.
>
> Perhaps there could be a way to customize vc-git so that it caters to
> such advanced users?

vc-git-resolve-conflicts is already a user option.

Were you thinking of something else?

[-- Attachment #2: Re: bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file.eml --]
[-- Type: message/rfc822, Size: 6700 bytes --]

From: Junio C Hamano <gitster@pobox.com>
To: 20292@debbugs.gnu.org
Cc: Dmitry Gutov <dgutov@yandex.ru>
Subject: Re: bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file
Date: Thu, 27 Oct 2016 10:20:52 -0700
Message-ID: <xmqqd1il7lor.fsf@gitster.mtv.corp.google.com>

Sorry for responding to an ancient message, but you said

> When a stash contains changes for several files, and "stash pop"
> encounters conflicts only in some of them, the rest of the files are
> stages automatically.
> 
> So then when we unstage the files which had conflicts after
> resolving those, the result is mixed. Which doesn't look right.

But this is a completely normal and designed way how conflicts are
resolved during any "mergy" operations in Git.  It is not limited to
"stash pop" (or "stash apply").  "git merge", "git cherry-pick",
"git revert", "git am -3" and even "git checkout -m another-branch"
share this feature.

When doing a mergy operation, some paths will merge cleanly and some
paths will have conflicts.  A conflicted path will

 - leave multiple stages in the index; the common ancestor version
   in stage#1, the version you originally had in stage#2, and the
   version the mergy operation wanted to bring the changes from into
   your state in stage#3.

 - have conflicted merge result in the working tree, with the
   conflict markers.

The way Git is designed to be used is for the user to edit the
latter to come up with a conflict resolution in the working tree,
then add that result to the index, after the user is satisfied with
it, before continuing (typically, but not necessarily, to record the
result with "git commit").

Now, what about the cleanly automerged paths?  They are added to the
index and this is by design.  You can see why this is necessary by
doing:

 - Start a mergy operation (e.g.  "stash pop", or "git merge") that
   conflicts;

 - Resolve the conflicts in the working tree,

 - But before doing "git add" to tell Git that you are done with
   these paths, run "git diff" (no other parameters).  You may have
   to (setq vc-git-resolve-conflicts nil) for that

You will notice that this invocation of "git diff" show concisely
how the result of your conflict resolution differs from either of
the branch.  This is used by Git users as one of the final step of
verification before telling Git "I now am done with the conflict in
this path and resolution is good" by issuing "git add" for the path.

You will also notice that this invocation of "git diff" does not
clutter with changes that have been auto-merged cleanly.  At this
step of the workflow to resolve conflicts, the user is concentrating
on the paths that had conflicts, and it is crucial that cleanly
auto-merged paths do not get in the way.  The user can still view
the overall picture by asking "git diff HEAD" (to see both
automerged result and hand-resolved result, the latter of which may
or may not have been added to the index) and "git diff --cached" (to
see automerged result and hand-resolved and then "git add"'ed
result).

So, there is no "Bad news, everybody!" in the behaviour you
observed.

> What shall we do? Unstage the automatically-staged files? Revert the
> changes from this bug? It seems Git really wants the changes staged
> after the conflict resolution.

I do not know what you thought you can achieve by "unstage the
auto-merged paths?" here, but perhaps you were expecting "git diff"
(no arguments) would be the way to view all the changes that a mergy
operation with conflicted states brought in?  If that (i.e. "view
all the changes") was what you wanted to achieve, then neither
"unstage the auto-merged paths" nor "automatically 'git add' upon
saving the buffer" is a good solution.  

I was told by somebody that the message I am responding to
eventually resulted in vc-git-resolve-conflicts that defaults to
true in Emacs 25.1, which lead to "automatically 'git add' upon
saving the buffer".  I think this variable and its default is a big
disservice to Git users' whose daily work involves lots of conflict
resolutions in mergy operations.

Running "git diff HEAD" instead of "git diff" may be the solution,
if the problem were "I want to view all the changes, both already
added to the index and the ones that have not been yet", though I
am not sure from the "Bad news, everybody!" message if that is the
problem you were trying to solve.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: gitster@pobox.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 17 Nov 2016 00:45:43 +0200
> 
> > I don't even see the message you are responding to.  Was it sent only
> > to you, Dmitry?
> 
> It was addressed to both me and the bug email, but didn't arrive at the 
> latter. I'm attaching the full message here.

Thanks.  I don't understand why it is not on the tracker, I guess it
wasn't actually sent there, for some reason.

> >> You, as a core Git developer, represent the informed minority, and as
> >> such, it's probably not too much to expect that you and the others can
> >> customize vc-git-resolve-conflicts to nil without much trouble.
> >
> > Perhaps there could be a way to customize vc-git so that it caters to
> > such advanced users?
> 
> vc-git-resolve-conflicts is already a user option.

My impression was that Juno, for some reason, finds that
insufficient.  But if I misunderstood, and the issue is only with its
default value, then indeed I see no problem.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 17.11.2016 17:57, Eli Zaretskii wrote:

> Thanks.  I don't understand why it is not on the tracker, I guess it
> wasn't actually sent there, for some reason.

The bug had been archived up to the same day. Although the request to 
unarchive it was sent before the email in question.

Or maybe the time zones conspired to make that false.

>> vc-git-resolve-conflicts is already a user option.
>
> My impression was that Juno, for some reason, finds that
> insufficient.  But if I misunderstood, and the issue is only with its
> default value, then indeed I see no problem.

He indeed complained about the default value.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

From the shell prompt type "git stash save" to stash uncommitted
changes.  Then "git pull" or "git merge" from upstream, and finally
type "git stash pop" to restore your original changes.

If "git stash pop" encounters merge conflicts, then resolving these
conflicts in Emacs and saving the buffer will run "git add" for the
file whose conflicts were resolved.  But that is not TRT in this case;
the user likely does not expect to have her uncommitted changes staged
for the next commit.

Therefore, I think vc-git-resolve-when-done should not run "git add"
if the merge conflict was due to "git stash pop".



In GNU Emacs 24.5.1 (i686-pc-mingw32)
 of 2015-03-27 on HOME-C4E4A596F7
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --prefix=/d/usr 'CFLAGS=-Og -gdwarf-4 -g3''

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: RMAIL

Minor modes in effect:
  diff-auto-refine-mode: t
  desktop-save-mode: t
  show-paren-mode: t
  display-time-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  line-number-mode: t

Recent messages:
Getting mail from d:/usr/eli/data/mail.new...
Counting new messages...done (12)
Saving file d:/usr/eli/rmail/INBOX...
Wrote d:/usr/eli/rmail/INBOX [2 times]
Computing summary lines...done
12 new messages read
Mark set
No following nondeleted message
Saving file d:/usr/eli/rmail/INBOX...
Wrote d:/usr/eli/rmail/INBOX [2 times]

Load-path shadows:
d:/usr/share/emacs/site-lisp/soap-inspect hides d:/usr/share/emacs/24.5/lisp/net/soap-inspect
d:/usr/share/emacs/site-lisp/soap-client hides d:/usr/share/emacs/24.5/lisp/net/soap-client

Features:
(shadow emacsbug etags smerge-mode cc-awk bug-reference add-log
misearch multi-isearch dabbrev shr-color color mule-util rmailout shr
browse-url network-stream starttls tls mail-extr smtpmail auth-source
eieio byte-opt bytecomp byte-compile cconv eieio-core password-cache
mailalias sendmail help-mode cl-extra texinfo ld-script tcl conf-mode
org-element org-rmail org-mhe org-irc org-info org-gnus gnus-util
org-docview doc-view image-mode dired-x dired org-bibtex bibtex
org-bbdb org-w3m org advice org-macro org-footnote org-pcomplete
pcomplete org-list org-faces org-entities org-version ob-emacs-lisp ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint
comint ansi-color ring ob-core ob-eval org-compat org-macs
org-loaddefs find-func cal-menu calendar cal-loaddefs arc-mode
archive-mode parse-time diff-mode vc-bzr sh-script smie executable
generic make-mode noutline outline easy-mmode vc-cvs jka-compr
bat-mode vc-dispatcher vc-svn flyspell info vc-git cc-langs rmailsum
qp rmailmm message format-spec rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mailabbrev gmm-utils mailheader mail-parse rfc2231 rmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils
desktop frameset server filecache mairix cus-edit cus-start cus-load
wid-edit cl-loaddefs cl-lib saveplace midnight ispell generic-x
cc-mode cc-fonts easymenu cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs paren battery time time-date tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32
ls-lisp w32-common-fns disp-table w32-win w32-vars tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
w32notify w32 multi-tty emacs)

Memory information:
((conses 8 1513834 148516)
 (symbols 32 42131 0)
 (miscs 32 5809 4393)
 (strings 16 105774 18210)
 (string-bytes 1 3133483)
 (vectors 8 37368)
 (vector-slots 4 1563878 83782)
 (floats 8 326 1158)
 (intervals 28 269674 1601)
 (buffers 508 358))

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/10/2015 03:55 PM, Eli Zaretskii wrote:

> If "git stash pop" encounters merge conflicts, then resolving these
> conflicts in Emacs and saving the buffer will run "git add" for the
> file whose conflicts were resolved.  But that is not TRT in this case;
> the user likely does not expect to have her uncommitted changes staged
> for the next commit.

Apparently, to both mark the conflict as resolved and not stage the 
file, the best we can do is 'git add ...; git reset ...', which would 
not DTRT if the file had some changes, and they were staged before you 
did 'git stash pop' (if the file had unstaged changes, 'git stash pop' 
would abort).

Should we be concerned about that?

> Therefore, I think vc-git-resolve-when-done should not run "git add"
> if the merge conflict was due to "git stash pop".

Maybe we can detect this case (as well as any similar ones) by the 
absence of .git/MERGE_HEAD.

But what's the justification for vc-git-resolve-when-done?

I think vc-git-checkin will work well enough without that.

Further, if there's a conflict, 'git stash pop' doesn't actually remove 
the stash from the list. Would we expect vc-git-resolve-when-done to 
call 'git stash drop' at the end of conflict resolution?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sat, 18 Apr 2015 22:16:51 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: "Eric S. Raymond" <esr@snark.thyrsus.com>
> 
> On 04/10/2015 03:55 PM, Eli Zaretskii wrote:
> 
> > If "git stash pop" encounters merge conflicts, then resolving these
> > conflicts in Emacs and saving the buffer will run "git add" for the
> > file whose conflicts were resolved.  But that is not TRT in this case;
> > the user likely does not expect to have her uncommitted changes staged
> > for the next commit.
> 
> Apparently, to both mark the conflict as resolved and not stage the 
> file, the best we can do is 'git add ...; git reset ...', which would 
> not DTRT if the file had some changes, and they were staged before you 
> did 'git stash pop' (if the file had unstaged changes, 'git stash pop' 
> would abort).

It's best not to run "git add" in the first place in this case.

> > Therefore, I think vc-git-resolve-when-done should not run "git add"
> > if the merge conflict was due to "git stash pop".
> 
> Maybe we can detect this case (as well as any similar ones) by the 
> absence of .git/MERGE_HEAD.

Why not detect that the conflict was from stashed changes?  This is
clearly stated at the last conflict marker.  The find-file-hook could
detect that and record the information.

> But what's the justification for vc-git-resolve-when-done?

So that "git commit" would "just work", I presume.

> I think vc-git-checkin will work well enough without that.

That would mean VC behaves wit Git differently than it does with other
VCSes (bzr, at least).

> Further, if there's a conflict, 'git stash pop' doesn't actually remove 
> the stash from the list. Would we expect vc-git-resolve-when-done to 
> call 'git stash drop' at the end of conflict resolution?

Yes.  Although IME, Git itself does that when you resolve the last
conflict.  But I'm not going to claim that this is 100% accurate, just
that it happened to me when I needed to resolve conflicts from stash.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/18/2015 10:31 PM, Eli Zaretskii wrote:

> It's best not to run "git add" in the first place in this case.

How will we detect it? And why would the user expect this difference in 
behavior? They'd either have a file nicely resolved, or the conflict 
unresolved, *and* a part of changes in staging area?

> Why not detect that the conflict was from stashed changes?  This is
> clearly stated at the last conflict marker.  The find-file-hook could
> detect that and record the information.

It's more complicated, but sounds better if we prefer to detect 
unstashing specifically, as opposed to any conflicts that were created 
by a non-merge operation, I guess.

>> But what's the justification for vc-git-resolve-when-done?
>
> So that "git commit" would "just work", I presume.

A lot of problems start with someone wanting to make something "just work".

> That would mean VC behaves wit Git differently than it does with other
> VCSes (bzr, at least).

You mean smerge-mode, not VC, right? How come? I don't even see

> Yes.

What if the user called 'git stash apply' instead of 'git stash pop'?

> Although IME, Git itself does that when you resolve the last
> conflict.  But I'm not going to claim that this is 100% accurate, just
> that it happened to me when I needed to resolve conflicts from stash.

I didn't when I tried it, a couple of times.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 12:58 AM, Dmitry Gutov wrote:

>> That would mean VC behaves wit Git differently than it does with other
>> VCSes (bzr, at least).
>
> You mean smerge-mode, not VC, right? How come? I don't even see

   ^^^ Sorry, please disregard this part. I can see it all right.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 00:58:47 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> On 04/18/2015 10:31 PM, Eli Zaretskii wrote:
> 
> > It's best not to run "git add" in the first place in this case.
> 
> How will we detect it?

I suggested one method below; perhaps there are others, I simply don't
know enough about Git.

> And why would the user expect this difference in behavior? They'd
> either have a file nicely resolved, or the conflict unresolved,
> *and* a part of changes in staging area?

Stashed changes were uncommitted before, so they should stay
uncommitted after, I think.  Having them staged means the situation
after "stash pop" is different than it was before "stash save", which
I think is not what the user expects.

> > Why not detect that the conflict was from stashed changes?  This is
> > clearly stated at the last conflict marker.  The find-file-hook could
> > detect that and record the information.
> 
> It's more complicated, but sounds better if we prefer to detect 
> unstashing specifically, as opposed to any conflicts that were created 
> by a non-merge operation, I guess.

If there is a better way of doing that, fine.

> >> But what's the justification for vc-git-resolve-when-done?
> >
> > So that "git commit" would "just work", I presume.
> 
> A lot of problems start with someone wanting to make something "just work".

But sometimes "just works" is not a beginning of a problem.

> What if the user called 'git stash apply' instead of 'git stash pop'?

If you are questioning the wisdom of doing "stash drop", then this
question is not for me: it wasn't my suggestion.  If we are not sure
dropping the stash automatically is what the user wants, let's not
drop it, and leave management of stashes to the user.  It's not a big
deal to leave the stash behind, I think.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 05:30 PM, Eli Zaretskii wrote:

> I suggested one method below; perhaps there are others, I simply don't
> know enough about Git.

Apparently, we misunderstand each other. By "this case", do you mean 
when merging a stash in general?

Because I've described a more specific case (popping a stash when one 
has staged changes in one of the involved files), and it looked like you 
were referring to it in >>best not to run "git add" in the first place<<.

> Stashed changes were uncommitted before, so they should stay
> uncommitted after, I think.  Having them staged means the situation
> after "stash pop" is different than it was before "stash save", which
> I think is not what the user expects.

Right. And I meant the difference between what we do depending on 
whether user has something staged originally.

> If you are questioning the wisdom of doing "stash drop", then this
> question is not for me: it wasn't my suggestion.

You said "yes". I asked about this in the context of consistency; the 
question was about how far will we go to be consistent with Bzr, and 
whether it's feasible to do so, or we should stop at some point.

> If we are not sure
> dropping the stash automatically is what the user wants, let's not
> drop it, and leave management of stashes to the user.  It's not a big
> deal to leave the stash behind, I think.

It's not that big a deal to leave marking files as resolved to the user 
either. Am I right to understand that's what you're currently 
suggesting, at least when dealing with stashes?

This is easy (so, done).

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 19:28:40 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> On 04/19/2015 05:30 PM, Eli Zaretskii wrote:
> 
> > I suggested one method below; perhaps there are others, I simply don't
> > know enough about Git.
> 
> Apparently, we misunderstand each other. By "this case", do you mean 
> when merging a stash in general?

I meant when "git stash pop" reports conflicts, in particular after a
"git pull" or "git merge".

> Because I've described a more specific case (popping a stash when one 
> has staged changes in one of the involved files), and it looked like you 
> were referring to it in >>best not to run "git add" in the first place<<.

I think we were talking about the same use case, but I cannot be sure,
since "has staged changes" might me more general than what I had in
mind.

> > Stashed changes were uncommitted before, so they should stay
> > uncommitted after, I think.  Having them staged means the situation
> > after "stash pop" is different than it was before "stash save", which
> > I think is not what the user expects.
> 
> Right. And I meant the difference between what we do depending on 
> whether user has something staged originally.

Before "git stash save"?  The case I had in mind didn't have anything
staged before that.

> > If you are questioning the wisdom of doing "stash drop", then this
> > question is not for me: it wasn't my suggestion.
> 
> You said "yes".

Yes, because someone more knowledgeable than myself said it was a good
idea.

> I asked about this in the context of consistency; the question was
> about how far will we go to be consistent with Bzr, and whether it's
> feasible to do so, or we should stop at some point.

I think it's okay to leave the stash and not drop it in this case.

> > If we are not sure
> > dropping the stash automatically is what the user wants, let's not
> > drop it, and leave management of stashes to the user.  It's not a big
> > deal to leave the stash behind, I think.
> 
> It's not that big a deal to leave marking files as resolved to the user 
> either. Am I right to understand that's what you're currently 
> suggesting, at least when dealing with stashes?

What does it mean to "mark files as resolved" when the conflict comes
from stashed changes that were uncommitted before "stash save"?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 08:06 PM, Eli Zaretskii wrote:

> I meant when "git stash pop" reports conflicts, in particular after a
> "git pull" or "git merge".

That's the general case.

> I think we were talking about the same use case, but I cannot be sure,
> since "has staged changes" might me more general than what I had in
> mind.

No: it's more specific. Before you do 'git stash pop', either there's 
nothing in the staging area (and the conflict is most likely due to some 
new commits), or there is something in the staging area.

"one has staged changes in one of the involved files" means the latter.

> Before "git stash save"?  The case I had in mind didn't have anything
> staged before that.

No, after "git stash save", but before "git stash pop".

> Yes, because someone more knowledgeable than myself said it was a good
> idea.

It was a question. :)

> What does it mean to "mark files as resolved" when the conflict comes
> from stashed changes that were uncommitted before "stash save"?

Changes that were staged, but then put into stash is a whole new 
different wrinkle. Luckily, 'git stash pop' only concerns itself with 
that distinction when passed a '--index' argument.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 20:38:30 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> On 04/19/2015 08:06 PM, Eli Zaretskii wrote:
> 
> > I meant when "git stash pop" reports conflicts, in particular after a
> > "git pull" or "git merge".
> 
> That's the general case.
> 
> > I think we were talking about the same use case, but I cannot be sure,
> > since "has staged changes" might me more general than what I had in
> > mind.
> 
> No: it's more specific. Before you do 'git stash pop', either there's 
> nothing in the staging area (and the conflict is most likely due to some 
> new commits), or there is something in the staging area.
> 
> "one has staged changes in one of the involved files" means the latter.
> 
> > Before "git stash save"?  The case I had in mind didn't have anything
> > staged before that.
> 
> No, after "git stash save", but before "git stash pop".

I was talking about the following simple scenatio:

  git pull
  # get an error about uncommitted changes that prevent merge
  git stash save
  git pull
  git stash pop
  # get an error message about conflicts
  # resolve conflicts and save de-conflicted files

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 09:05 PM, Eli Zaretskii wrote:

> I was talking about the following simple scenatio:
>
>    git pull
>    # get an error about uncommitted changes that prevent merge
>    git stash save
>    git pull
>    git stash pop
>    # get an error message about conflicts
>    # resolve conflicts and save de-conflicted files

And I, about a related but slightly different one, which we'd prefer not 
to make worse, right?

     # edit foo.bar
     git stash save
     # edit foo.bar again, differently, maybe in several places
     git add foo.bar
     git stash pop
     # get an error message about conflict in foo.bar
     # resolve conflicts and save foo.bar

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 21:11:49 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> And I, about a related but slightly different one, which we'd prefer not 
> to make worse, right?
> 
>      # edit foo.bar
>      git stash save
>      # edit foo.bar again, differently, maybe in several places
>      git add foo.bar
>      git stash pop
>      # get an error message about conflict in foo.bar
>      # resolve conflicts and save foo.bar

If we can, yes.  If not, this is way out of scope of the bug report.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 09:25 PM, Eli Zaretskii wrote:

> If we can, yes.  If not, this is way out of scope of the bug report.

Do you like the solution in d35f2f482273a822df695202f4a3bf1a5e473e63?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 21:30:30 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> On 04/19/2015 09:25 PM, Eli Zaretskii wrote:
> 
> > If we can, yes.  If not, this is way out of scope of the bug report.
> 
> Do you like the solution in d35f2f482273a822df695202f4a3bf1a5e473e63?

IMO, it's too radical: there's no need to avoid turning on
smerge-mode, as it is useful for conflict resolution.  "git add" is
not run by smerge-mode, it is run by vc-git-resolve-when-done, so it
should be enough to avoid adding that to after-save-hook, I think.

Thanks.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/19/2015 09:38 PM, Eli Zaretskii wrote:

> IMO, it's too radical: there's no need to avoid turning on
> smerge-mode, as it is useful for conflict resolution.  "git add" is
> not run by smerge-mode, it is run by vc-git-resolve-when-done, so it
> should be enough to avoid adding that to after-save-hook, I think.

Right. That should be resolved now.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Sun, 19 Apr 2015 22:27:58 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> 
> On 04/19/2015 09:38 PM, Eli Zaretskii wrote:
> 
> > IMO, it's too radical: there's no need to avoid turning on
> > smerge-mode, as it is useful for conflict resolution.  "git add" is
> > not run by smerge-mode, it is run by vc-git-resolve-when-done, so it
> > should be enough to avoid adding that to after-save-hook, I think.
> 
> Right. That should be resolved now.

Thanks, looks good to me.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

>> If we can, yes.  If not, this is way out of scope of the bug report.
> Do you like the solution in d35f2f482273a822df695202f4a3bf1a5e473e63?

My main use case is "git stash pop; then repeatedly call M-x
vc-find-conflicted-files and resolve the conflicts (mostly with smerge's
help)".  I get the impression that with this change
vc-find-conflicted-files won't work correctly any more, because the
files will keep appearing as "conflicted", even after I've resolved all
the conflicts in them.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Eli Zaretskii <eliz@gnu.org>, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> Date: Sun, 19 Apr 2015 22:41:18 -0400
> 
> I get the impression that with this change vc-find-conflicted-files
> won't work correctly any more, because the files will keep appearing
> as "conflicted", even after I've resolved all the conflicts in them.

You need to "unconflict" them by hand, yes, with "git reset HEAD
FILE".  If it is safe to issue this command automatically, we could do
that in the case of "stash pop" conflict _instead_ of doing "git add"
in the "normal" merge-conflict case.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

>> I get the impression that with this change vc-find-conflicted-files
>> won't work correctly any more, because the files will keep appearing
>> as "conflicted", even after I've resolved all the conflicts in them.
> You need to "unconflict" them by hand, yes, with "git reset HEAD FILE".

I very much want Emacs to do that for me (or "git add", I really
couldn't care less which is used, I just want the "conflicted" marker to
be cleared).
I went to the trouble to get this working for Bzr back in the day, then
Svn (and managed to find someone else to do it for Git), because I find
it much too inconvenient otherwise.

> If it is safe to issue this command automatically, we could do
> that in the case of "stash pop" conflict _instead_ of doing "git add"
> in the "normal" merge-conflict case.

I don't really know what is "safe" in this respect.  For my own use, all
of the options we've considered are safe.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: dgutov@yandex.ru,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Mon, 20 Apr 2015 15:20:28 -0400
> 
> > If it is safe to issue this command automatically, we could do
> > that in the case of "stash pop" conflict _instead_ of doing "git add"
> > in the "normal" merge-conflict case.
> 
> I don't really know what is "safe" in this respect.  For my own use, all
> of the options we've considered are safe.

Then my vote is for using "git add FILE" with conflicts during a
merge, and "git reset HEAD FILE" with conflicts during "stash pop".  I
think this is the simplest solution and is easy to implement with
minimal changes.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> Then my vote is for using "git add FILE" with conflicts during a
> merge, and "git reset HEAD FILE" with conflicts during "stash pop".
> I think this is the simplest solution and is easy to implement with
> minimal changes.

Fine by me,


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 04/21/2015 04:06 AM, Stefan Monnier wrote:
>> Then my vote is for using "git add FILE" with conflicts during a
>> merge, and "git reset HEAD FILE" with conflicts during "stash pop".
>> I think this is the simplest solution and is easy to implement with
>> minimal changes.
>
> Fine by me,

I've pushed this, together with the choosing logic suggested previously. 
Apparently it's the best we can do.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Wed, 22 Apr 2015 04:50:14 +0300
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: esr@snark.thyrsus.com, 20292-done@debbugs.gnu.org
> 
> On 04/21/2015 04:06 AM, Stefan Monnier wrote:
> >> Then my vote is for using "git add FILE" with conflicts during a
> >> merge, and "git reset HEAD FILE" with conflicts during "stash pop".
> >> I think this is the simplest solution and is easy to implement with
> >> minimal changes.
> >
> > Fine by me,
> 
> I've pushed this, together with the choosing logic suggested previously. 
> Apparently it's the best we can do.

Thanks, this does TRT in my use cases.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

Bad news, everyone!

When a stash contains changes for several files, and "stash pop" 
encounters conflicts only in some of them, the rest of the files are 
stages automatically.

At least, that happens with Git 2.1.0 on my machine, and some commenters 
here: http://stackoverflow.com/a/1237337/615245

So then when we unstage the files which had conflicts after resolving 
those, the result is mixed. Which doesn't look right.

What shall we do? Unstage the automatically-staged files? Revert the 
changes from this bug? It seems Git really wants the changes staged 
after the conflict resolution.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> What shall we do? Unstage the automatically-staged files? Revert the changes
> from this bug? It seems Git really wants the changes staged after the
> conflict resolution.

IOW, the "resolve" step should always just use "git add", like we used
to do.
Yes, I think it's the right solution.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Wed, 13 May 2015 09:04:16 -0400
> 
> > What shall we do? Unstage the automatically-staged files? Revert the changes
> > from this bug? It seems Git really wants the changes staged after the
> > conflict resolution.
> 
> IOW, the "resolve" step should always just use "git add", like we used
> to do.

How is that TRT when the original state was that there were
uncommitted and unstaged changes?  Running "git add" would confuse
people who are not very knowledgeable about the index.  If they _are_
knowledgeable, they'll "git reset" right away anyway.

So please don't go back.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 13 May 2015 02:13:10 +0300
> 
> When a stash contains changes for several files, and "stash pop" encounters conflicts only in some of them, the rest of the files are stages automatically.
> 
> At least, that happens with Git 2.1.0 on my machine

I see this with 1.9.5 as well.

> What shall we do?

Report a bug in Git, I think.  It doesn't make sense to have the
outcome of "stash pop" wrt the index/staging depend on whether there
were conflicts or not, which is what happening here: if I "stash pop"
after pulling when none of my local stashed changes are in conflict
with the pulled/merged content, I get back modified and unstaged
files.  Why would the existence of conflicts during "stash pop"
produce any different effect for files _without_ conflicts, except by
some obscure bug?

> Unstage the automatically-staged files?

If we can do that, yes.  But how do we know which files to unstage?

> Revert the changes from this bug?

No!  That'd be a step back.  The current treatment of stashed changes
is better than it was before the change.  Also note that conflicts
like this are quite rare, so the way vc-git worked previously was
wrong in 99% of cases, why the one we have now is wrong in perhaps
0.1%.

> It seems Git really wants the changes staged after the conflict resolution.

It seems to me we've uncovered a bug in Git (gasp!).  Git has no
reasons to want the changes staged, certainly not depending on whether
there were conflicts.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/13/2015 07:18 PM, Eli Zaretskii wrote:

> Report a bug in Git, I think.

I believe it's your turn to report a Git bug now. ;)

Still, even if it's fixed, we'll have a lot of users, for years to come, 
that use Git without that fix. Note how you and I are using quite 
different versions, and the latest release is 2.4.1.

 > It doesn't make sense to have the
> outcome of "stash pop" wrt the index/staging depend on whether there
> were conflicts or not, which is what happening here: if I "stash pop"
> after pulling when none of my local stashed changes are in conflict
> with the pulled/merged content, I get back modified and unstaged
> files.  Why would the existence of conflicts during "stash pop"
> produce any different effect for files _without_ conflicts, except by
> some obscure bug?

As a wild guess, maybe the files that get staged automatically are the 
result of automatic conflict resolution (there was some divergence, but 
it was resolved automatically; maybe the "no divergence" case is also 
treated like this, for simplicity). IOW, the "worse is better" kind of 
reasons.

>> Unstage the automatically-staged files?
>
> If we can do that, yes.  But how do we know which files to unstage?

That the the difficulty: right after applying the stash we could know 
(all of them!), but Emacs can't know whether the user staged anything 
else between then and now (when all conflicts have been resolved). IOW, 
the user is better positioned to call 'git reset'.

> No!  That'd be a step back.  The current treatment of stashed changes
> is better than it was before the change.  Also note that conflicts
> like this are quite rare, so the way vc-git worked previously was
> wrong in 99% of cases, why the one we have now is wrong in perhaps
> 0.1%.

I don't know where you got the percentages. My stashes routinely touch 
multiple files, and it's easy to imagine how not all of them could have 
conflicts after applying.

The odds are hard to calculate, but the probability really must be in 
tens of percents, not below one.

The current behavior is bad because it looks random. It would look 
especially random to someone who's used to interact with Git via 
command-line.

> It seems to me we've uncovered a bug in Git (gasp!).  Git has no
> reasons to want the changes staged, certainly not depending on whether
> there were conflicts.

Staging changes is the Git way to mark conflict as resolved. Ergo, it 
expects the conflicting files to be staged after the user resolves the 
conflicts. Then it won't make a lot of sense to leave the rest of the 
files unstaged, would it? Maybe that's the reasoning.

It's hard for me to tell without knowing exactly why Git conflates 
conflict resolution and staging.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 04:24:12 +0300
> 
>     Report a bug in Git, I think.
> 
> I believe it's your turn to report a Git bug now. ;)

So we do turns on this?

> Still, even if it's fixed, we'll have a lot of users, for years to come, that use Git without that fix. Note how you and I are using quite different versions, and the latest release is 2.4.1.

I only have an old version because there's no newer one for Windows,
and I can't be bothered enough to build my own.

Anyway, the fact that it takes a long time for a fix to percolate
shouldn't preclude us from reporting it.

> > It doesn't make sense to have the
> > outcome of "stash pop" wrt the index/staging depend on whether there
> > were conflicts or not, which is what happening here: if I "stash pop"
> > after pulling when none of my local stashed changes are in conflict
> > with the pulled/merged content, I get back modified and unstaged
> > files.  Why would the existence of conflicts during "stash pop"
> > produce any different effect for files _without_ conflicts, except by
> > some obscure bug?
> 
> As a wild guess, maybe the files that get staged automatically are the result of automatic conflict resolution (there was some divergence, but it was resolved automatically; maybe the "no divergence" case is also treated like this, for simplicity). IOW, the "worse is better" kind of reasons.

No, I reproduced this when some of the stashed files were not changed
at all upstream, i.e. there shouldn't have been a need for any
conflict resolution, automatic or otherwise, in those files.

_My_ wild guess is that Git simply invokes the same code as is used in
a "normal" merge, and that one stages files that are without conflicts.

>         Unstage the automatically-staged files?
> 
>     If we can do that, yes.  But how do we know which files to unstage?
> 
> That the the difficulty: right after applying the stash we could know (all of them!), but Emacs can't know whether the user staged anything else between then and now (when all conflicts have been resolved). IOW, the user is better positioned to call 'git reset'.

The user is always better positioned, but we'd like VC to DTRT in the
more popular situations where the user could be saved from the
nuisance of figuring things out and typing shell commands.  That's the
main goal of VC, isn't it?

If we know all the stashed files, how about invoking "git reset" for
all of them?  It cannot hurt, can it?

>     No!  That'd be a step back.  The current treatment of stashed changes
>     is better than it was before the change.  Also note that conflicts
>     like this are quite rare, so the way vc-git worked previously was
>     wrong in 99% of cases, why the one we have now is wrong in perhaps
>     0.1%.
> 
> I don't know where you got the percentages. My stashes routinely touch multiple files, and it's easy to imagine how not all of them could have conflicts after applying.

I'm talking about conflicts, not about the number of files.  How many
times did you have conflicts in "stash pop"?  I almost never have
them.

> The odds are hard to calculate, but the probability really must be in tens of percents, not below one.

Now I wonder where did _you_ get your percentages.

> The current behavior is bad because it looks random.

I agree it's bad, but only if (a) there are multiple changed files,
and (b) some, but not all, of them have conflicts.  Otherwise, the
behavior is correct.  By contrast, the previous behavior was always
wrong.

>     It seems to me we've uncovered a bug in Git (gasp!).  Git has no
>     reasons to want the changes staged, certainly not depending on whether
>     there were conflicts.
> 
> Staging changes is the Git way to mark conflict as resolved.

Not for uncommitted changes that were stashed, it ain't.  For "normal"
merge conflicts, yes, because a conflict-free merge would have
committed the changes, so staging is a step in the right direction.
But for conflicts in stashed uncommitted changes, it's a step in the
wrong direction, especially in files that didn't have conflicts at
all.

> Ergo, it expects the conflicting files to be staged after the user resolves the conflicts. Then it won't make a lot of sense to leave the rest of the files unstaged, would it? Maybe that's the reasoning.

It's a flawed reasoning, IMO.  I stashed the changes because they are
not yet ready to be committed, and I wanted them out of my way for a
while.  When I pop the stash, I want them uncommitted as they were
before.  Having them staged means that I might inadvertently commit
them with my next "git commit", something that wasn't supposed to
happen before the stash.

> It's hard for me to tell without knowing exactly why Git conflates conflict resolution and staging.

It's a bug.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 05:53 PM, Eli Zaretskii wrote:

> So we do turns on this?

Why not? You seem to have a better idea why this behavior is necessarily 
a bug.

> I only have an old version because there's no newer one for Windows,
> and I can't be bothered enough to build my own.

Then it'll even more likely that a lot of users will be on the "old 
version".

> Anyway, the fact that it takes a long time for a fix to percolate
> shouldn't preclude us from reporting it.

Hopefully, if and when there's a fix, Emacs's behavior won't have to 
change much. But if Git grows a different "resolve a conflict" workflow, 
we'll try to honor it.

> No, I reproduced this when some of the stashed files were not changed
> at all upstream, i.e. there shouldn't have been a need for any
> conflict resolution, automatic or otherwise, in those files.

Then the guess from the end of the message you're replying to, might be 
closer to the truth.

> _My_ wild guess is that Git simply invokes the same code as is used in
> a "normal" merge, and that one stages files that are without conflicts.

Right, or that.

> The user is always better positioned, but we'd like VC to DTRT in the
> more popular situations where the user could be saved from the
> nuisance of figuring things out and typing shell commands.  That's the
> main goal of VC, isn't it?

If we can do that without introducing inconsistencies, losing 
information, or surprising a lot of users.

> If we know all the stashed files, how about invoking "git reset" for
> all of them?  It cannot hurt, can it?

How will we know it? Emacs could try to list all staged files, but 
there's no good way to know that they all belong to the applied stash 
(looking at the top stash isn't reliable either: the user might have 
specified a different one explicitly).

> I'm talking about conflicts, not about the number of files.  How many
> times did you have conflicts in "stash pop"?

Often. But that's irrelevant: in all cases when we don't have a conflict 
when applying a stash, this bug does not apply.

So we should be discussing the percentage of "conflict in only some of 
the files" out of "conflict when applying the stash" situations.

>> The odds are hard to calculate, but the probability really must be in tens of percents, not below one.
>
> Now I wonder where did _you_ get your percentages.

I'm simply basing it on the assumption that a stash likely touches 
multiple files (and that depends on the project/language/environment, so 
it could be frequently false in certain old-school "a few files, each of 
them huge" C projects).

If the stash does touch several files, and there's a conflict, it's easy 
to imagine that the conflict would be only in some of them.

> I agree it's bad, but only if (a) there are multiple changed files,
> and (b) some, but not all, of them have conflicts.

Which is a fairly common situation, like described above.

> By contrast, the previous behavior was always
> wrong.

It was non-ideal, but apparently it was consistent with how a person 
usually works with Git.

>> Staging changes is the Git way to mark conflict as resolved.
>
> Not for uncommitted changes that were stashed, it ain't.

It is. Everywhere the documentation talks about resolving a conflict, 
the documented next step is 'git add'. Nowhere it talks about doing 
something else after resolving a stash conflict.

I'd love to be proved wrong, though.

> For "normal"
> merge conflicts, yes, because a conflict-free merge would have
> committed the changes, so staging is a step in the right direction.
> But for conflicts in stashed uncommitted changes, it's a step in the
> wrong direction, especially in files that didn't have conflicts at
> all.

Here you're talking about your own intention, not about the usual Git 
workflow. Yes, it might be suboptimal, but we might have to live with it 
anyway.

> It's a flawed reasoning, IMO.  I stashed the changes because they are
> not yet ready to be committed, and I wanted them out of my way for a
> while.  When I pop the stash, I want them uncommitted as they were
> before.

Sure, that's why it's suboptimal. But apparently at some point a 
decision was made to handle "normal" merge conflicts and the stash 
conflicts in the same way.

I may be wrong about this: the Git mailing list is a better place to ask.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 21:00:36 +0300
> 
> > If we know all the stashed files, how about invoking "git reset" for
> > all of them?  It cannot hurt, can it?
> 
> How will we know it? Emacs could try to list all staged files, but
> there's no good way to know that they all belong to the applied
> stash (looking at the top stash isn't reliable either: the user
> might have specified a different one explicitly).

We could assume that any staged file during resolution of conflict due
to "stash pop" is from the stash.  If that's too dangerous (is it?),
we could ask for confirmation.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> is better than it was before the change.  Also note that conflicts
> like this are quite rare,

It's about 99.9% of my "unstash-caused conflicts", so "rare" obviously
depends on how you use Git.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Wed, 13 May 2015 23:49:25 -0400
> 
> > is better than it was before the change.  Also note that conflicts
> > like this are quite rare,
> 
> It's about 99.9% of my "unstash-caused conflicts", so "rare" obviously
> depends on how you use Git.

Indeed.

May I ask why you have uncommitted changes when you pull (or do
whatever else requires to stash them)?  Why don't you commit them or
move them to a branch, or work on a branch to begin with?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> May I ask why you have uncommitted changes when you pull (or do
> whatever else requires to stash them)?

Typical case:
- before committing, I do a final "pull".
- the "pull" fails, so I have to stash/pull/unstash
- the commit touches several files and has a conflict somewhere.

> Why don't you commit them or move them to a branch, or work on
> a branch to begin with?

Old habits die hard,


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 06:51 PM, Stefan Monnier wrote:
>> May I ask why you have uncommitted changes when you pull (or do
>> whatever else requires to stash them)?

My main use case: there are local changes in several configuration 
files, which I don't ever intend to commit.

> Typical case:
> - before committing, I do a final "pull".
> - the "pull" fails, so I have to stash/pull/unstash
> - the commit touches several files and has a conflict somewhere.

To avoid this scenario, I usually commit, and then 'git pull --rebase', 
which we don't, and shouldn't, recommend.

>> Why don't you commit them or move them to a branch, or work on
>> a branch to begin with?

I think it would be annoying to create a branch for every tiny change.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 20:30:26 +0300
> 
> On 05/14/2015 06:51 PM, Stefan Monnier wrote:
> >> May I ask why you have uncommitted changes when you pull (or do
> >> whatever else requires to stash them)?
> 
> My main use case: there are local changes in several configuration 
> files, which I don't ever intend to commit.
> 
> > Typical case:
> > - before committing, I do a final "pull".
> > - the "pull" fails, so I have to stash/pull/unstash
> > - the commit touches several files and has a conflict somewhere.
> 
> To avoid this scenario, I usually commit, and then 'git pull --rebase', 
> which we don't, and shouldn't, recommend.
> 
> >> Why don't you commit them or move them to a branch, or work on
> >> a branch to begin with?
> 
> I think it would be annoying to create a branch for every tiny change.

So: do we have a way of knowing which files had their changes stashed?
Then we could call "git reset" on each one of them, after "stash pop".

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 09:36 PM, Eli Zaretskii wrote:

> So: do we have a way of knowing which files had their changes stashed?

I don't think so. Further, like I mentioned before, the user might have 
staged some further changes to those other, non-conflicting files.

Do you apply stashes via the vc-dir interface? We could do something in 
that implementation.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 21:48:21 +0300
> 
> Do you apply stashes via the vc-dir interface?

No.

> We could do something in that implementation.

I think its a good idea.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 09:52 PM, Eli Zaretskii wrote:

> I think its a good idea.

It might be. But I don't actually know how to approach it.

Doing 'git reset' right after a stash is popped would break 
vc-git-conflicted-files.

If we don't do that, then what else *do* we do in vc-git-stash-apply and 
vc-git-stash-pop?

Note that vc-git-resolve-when-done has to work satisfactorily both after 
vc-git-stash-apply, and after 'git stash apply' performed in console.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 22:09:39 +0300
> 
> On 05/14/2015 09:52 PM, Eli Zaretskii wrote:
> 
> > I think its a good idea.
> 
> It might be. But I don't actually know how to approach it.
> 
> Doing 'git reset' right after a stash is popped would break 
> vc-git-conflicted-files.
> 
> If we don't do that, then what else *do* we do in vc-git-stash-apply and 
> vc-git-stash-pop?

Ask the user whether to reset each file in the index (other than the
one in which the conflicts were resolved), perhaps?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 10:33 PM, Eli Zaretskii wrote:

> Ask the user whether to reset each file in the index (other than the
> one in which the conflicts were resolved), perhaps?

That doesn't sound to me like something an after-save-hook should do.

And we can't reset each other file, we'd first have to check whether 
some of them still conflicts.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

I think the only sane way to handle this is to always use "git add" and
to add a config var so users who don't like it can disable it.

After all, when unstashing changes in a file that already has
a modification staged, Git is pretty happy to silently/automatically
"git add" the resulting merge (hence throwing away the info about the
particular changes that were staged before the unstash) if it doesn't have
conflicts (at least it does so if the unstash finds conflicts in other
files).  So I don't see why Emacs shouldn't feel free to "git add" the
file once conflicts are resolved.

IOW, I think bug#20292 is simply not a bug.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Thu, 14 May 2015 16:55:50 -0400
> 
> 
> I think the only sane way to handle this is to always use "git add" and
> to add a config var so users who don't like it can disable it.

Who'd like it?  It will do something utterly inappropriate.

> After all, when unstashing changes in a file that already has
> a modification staged

That's not the use case we were discussing, though.  We were
discussing a use case where the user merged from another repository,
and then wants her uncommitted changes restored.  Leaving them staged
will trip the naive users.

> IOW, I think bug#20292 is simply not a bug.

I respectfully disagree.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> That's not the use case we were discussing, though.  We were
> discussing a use case where the user merged from another repository,
> and then wants her uncommitted changes restored.  Leaving them staged
> will trip the naive users.

But Emacs is not the main culprit: Git itself will stage all the
non-conflicting changes, so why should this not trip the user similarly?

IOW if the user gets tripped by Emacs doing "git add" after resolving
a unstash conflict, why would that same user not already be tripped
identically by Git doing this "git add" on the non-conflicted files?


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: dgutov@yandex.ru,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Fri, 15 May 2015 14:13:50 -0400
> 
> > That's not the use case we were discussing, though.  We were
> > discussing a use case where the user merged from another repository,
> > and then wants her uncommitted changes restored.  Leaving them staged
> > will trip the naive users.
> 
> But Emacs is not the main culprit: Git itself will stage all the
> non-conflicting changes, so why should this not trip the user similarly?

The users I have in mind expect Emacs to save them from Git
idiosyncrasies.

> IOW if the user gets tripped by Emacs doing "git add" after resolving
> a unstash conflict, why would that same user not already be tripped
> identically by Git doing this "git add" on the non-conflicted files?

Because they don't use Git from the shell, or at least try not to.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

>> > That's not the use case we were discussing, though.  We were
>> > discussing a use case where the user merged from another repository,
>> > and then wants her uncommitted changes restored.  Leaving them staged
>> > will trip the naive users.
>> But Emacs is not the main culprit: Git itself will stage all the
>> non-conflicting changes, so why should this not trip the user similarly?
> The users I have in mind expect Emacs to save them from Git
> idiosyncrasies.

I don't see how that's relevant.  By behaving differently from the rest
of Git, I'm afraid we'll just introduce more problems.

>> IOW if the user gets tripped by Emacs doing "git add" after resolving
>> a unstash conflict, why would that same user not already be tripped
>> identically by Git doing this "git add" on the non-conflicted files?
> Because they don't use Git from the shell, or at least try not to.

Feel free to change the behavior of vc-git-resolve-when-done for the
case where the unstash was done from within Emacs after you've changed
this unstash to behave the way you want it, rather than the way Git
does it.

In my case, the unstash is done by Git with no Emacs involvement, and in
that case it seems that "git add" is just the only sane thing to do.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: dgutov@yandex.ru,  esr@snark.thyrsus.com,  20292@debbugs.gnu.org
> Date: Fri, 15 May 2015 16:02:03 -0400
> 
> >> > That's not the use case we were discussing, though.  We were
> >> > discussing a use case where the user merged from another repository,
> >> > and then wants her uncommitted changes restored.  Leaving them staged
> >> > will trip the naive users.
> >> But Emacs is not the main culprit: Git itself will stage all the
> >> non-conflicting changes, so why should this not trip the user similarly?
> > The users I have in mind expect Emacs to save them from Git
> > idiosyncrasies.
> 
> I don't see how that's relevant.

Strange.

> By behaving differently from the rest of Git, I'm afraid we'll just
> introduce more problems.

I'm not afraid of that.

> >> IOW if the user gets tripped by Emacs doing "git add" after resolving
> >> a unstash conflict, why would that same user not already be tripped
> >> identically by Git doing this "git add" on the non-conflicted files?
> > Because they don't use Git from the shell, or at least try not to.
> 
> Feel free to change the behavior of vc-git-resolve-when-done for the
> case where the unstash was done from within Emacs after you've changed
> this unstash to behave the way you want it, rather than the way Git
> does it.
> 
> In my case, the unstash is done by Git with no Emacs involvement, and in
> that case it seems that "git add" is just the only sane thing to do.

Then I guess the only way to stop this endless and futile argument is
to have an option that will control whether we "add" or "reset".

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> Then I guess the only way to stop this endless and futile argument is
> to have an option that will control whether we "add" or "reset".

That sounds right (and is basically what I suggested, tho what
I suggested was a boolean to prevent "git add", but indeed we
could make it into a 3-way choice between "git add", "git reset",
and "do nothing").

If we want something more refined, I think we'd need to more precisely
characterize the cases where we want "git add" and those where we want
"git reset" (it seems many details are important such as whether the
conflict comes from "git merge" or from "git stash", whether there were
staged changes before the command was run, maybe more) and AFAIK those
cases can't be distinguished solely based on the state of the current
file but also depend on the other files in the project.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/16/2015 02:52 AM, Stefan Monnier wrote:

> but indeed we
> could make it into a 3-way choice between "git add", "git reset",
> and "do nothing").

You've read my mind. :)

> whether there were
> staged changes before the command was run

We'll just ignore this.

> and AFAIK those
> cases can't be distinguished solely based on the state of the current
> file but also depend on the other files in the project.

That's not a problem, we have vc-git-root.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

>> and AFAIK those cases can't be distinguished solely based on the
>> state of the current file but also depend on the other files in
>> the project.
> That's not a problem, we have vc-git-root.

It's not a theoretical problem, no, but it's a practical/programming
problem in that someone has to write the code, and the resulting code
will be that much more brittle and inefficient.


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/14/2015 11:55 PM, Stefan Monnier wrote:
>
> I think the only sane way to handle this is to always use "git add" and
> to add a config var so users who don't like it can disable it.

If we're fine with a config var, we might as well try to support the 
alternative behavior. This should do it:

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 20f2101..2fbb71b 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -130,6 +130,17 @@ If nil, use the value of `vc-annotate-switches'. 
If t, use no switches."
    :version "25.1"
    :group 'vc-git)

+(defcustom vc-git-resolve-conflicts t
+  "When non-nil, mark conflicted file as resolved upon saving.
+That happens after all conflict markers in it have been removed.
+If the value is `unstage', then after the last conflict is
+resolved, the staging area is cleared if no merge is in progress."
+  :type '(choice (const :tag "Don't resolve" nil)
+                 (const :tag "Resolve" t)
+                 (const :tag "Unstage all after resolving" unstage))
+  :version "25.1"
+  :group 'vc-git)
+
  (defcustom vc-git-program "git"
    "Name of the Git executable (excluding any arguments)."
    :version "24.1"
@@ -801,12 +812,16 @@ This prompts for a branch to merge from."
    (save-excursion
      (goto-char (point-min))
      (unless (re-search-forward "^<<<<<<< " nil t)
-      (if (file-exists-p (expand-file-name ".git/MERGE_HEAD"
-                                           (vc-git-root buffer-file-name)))
-          ;; Doing a merge.
-          (vc-git-command nil 0 buffer-file-name "add")
-        ;; Doing something else.  Likely applying a stash (bug#20292).
-        (vc-git-command nil 0 buffer-file-name "reset"))
+      (vc-git-command nil 0 buffer-file-name "add")
+      (when (and
+             (eq vc-git-resolve-conflicts 'unstage)
+             ;; Doing something other than a merge.  Likely applying a
+             ;; stash (bug#20292).
+             (not
+              (file-exists-p (expand-file-name ".git/MERGE_HEAD"
+                                               (vc-git-root 
buffer-file-name))))
+             (not (vc-git-conflicted-files (vc-git-root 
buffer-file-name))))
+        (vc-git-command nil 0 nil "reset"))
        ;; Remove the hook so that it is not called multiple times.
        (remove-hook 'after-save-hook 'vc-git-resolve-when-done t))))

@@ -823,7 +838,8 @@ This prompts for a branch to merge from."
                 (re-search-forward "^<<<<<<< " nil 'noerror)))
      (vc-file-setprop buffer-file-name 'vc-state 'conflict)
      (smerge-start-session)
-    (add-hook 'after-save-hook 'vc-git-resolve-when-done nil 'local)
+    (when vc-git-resolve-conflicts
+      (add-hook 'after-save-hook 'vc-git-resolve-when-done nil 'local))
      (message "There are unresolved conflicts in this file")))

  ;;; HISTORY FUNCTIONS

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 16 May 2015 02:50:57 +0300
> 
> On 05/14/2015 11:55 PM, Stefan Monnier wrote:
> >
> > I think the only sane way to handle this is to always use "git add" and
> > to add a config var so users who don't like it can disable it.
> 
> If we're fine with a config var, we might as well try to support the 
> alternative behavior. This should do it:

Thanks.  Just so I'm sure my reading of the patch is correct: this
option will never modify the behavior for conflicts that arise during
a "normal" merge (as opposed to "stash pop", for example), right?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/16/2015 10:15 AM, Eli Zaretskii wrote:

> Thanks.  Just so I'm sure my reading of the patch is correct: this
> option will never modify the behavior for conflicts that arise during
> a "normal" merge (as opposed to "stash pop", for example), right?

That's the intention, yes.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: monnier@iro.umontreal.ca, esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 16 May 2015 11:03:36 +0300
> 
> On 05/16/2015 10:15 AM, Eli Zaretskii wrote:
> 
> > Thanks.  Just so I'm sure my reading of the patch is correct: this
> > option will never modify the behavior for conflicts that arise during
> > a "normal" merge (as opposed to "stash pop", for example), right?
> 
> That's the intention, yes.

Great, thanks.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/16/2015 11:55 AM, Eli Zaretskii wrote:

> Great, thanks.

Pushed, with some tweaks.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Stefan Monnier]

> If we're fine with a config var, we might as well try to support the
> alternative behavior. This should do it:

Looks OK to me.

> +  :group 'vc-git)

This is redundant.

> +      (when (and
> +             (eq vc-git-resolve-conflicts 'unstage)
> +             ;; Doing something other than a merge.  Likely applying a
> +             ;; stash (bug#20292).
> +             (not
> +              (file-exists-p (expand-file-name ".git/MERGE_HEAD"
> +                                               (vc-git-root
> buffer-file-name))))
> +             (not (vc-git-conflicted-files (vc-git-root
> buffer-file-name))))

If you switch to "(unless (or ..." you'll get rid of one "not".


        Stefan

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Dmitry Gutov]

On 05/16/2015 04:53 PM, Stefan Monnier wrote:

> This is redundant.

> If you switch to "(unless (or ..." you'll get rid of one "not".

These should be addressed now.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 23:24:34 +0300
> 
> On 05/14/2015 10:33 PM, Eli Zaretskii wrote:
> 
> > Ask the user whether to reset each file in the index (other than the
> > one in which the conflicts were resolved), perhaps?
> 
> That doesn't sound to me like something an after-save-hook should do.

Why not?

> And we can't reset each other file, we'd first have to check whether 
> some of them still conflicts.

Can we know that conflicts still exist?

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Cc: esr@snark.thyrsus.com, 20292@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 May 2015 23:24:34 +0300
> 
> And we can't reset each other file, we'd first have to check whether 
> some of them still conflicts.

Btw, if we are seriously considering Stefan's suggestion to "git add"
after conflicts are resolved, I see no reason not to "git reset"
everything instead, since all that would do is make the staged files
unstaged -- hardly a disaster, as all the user needs to do is add them
back, and a user who knows Git enough to have staged changes before
un-stashing will have no problems with that (if they at all use VC).

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >> Then my vote is for using "git add FILE" with conflicts during a
  > >> merge, and "git reset HEAD FILE" with conflicts during "stash pop".
  > >> I think this is the simplest solution and is easy to implement with
  > >> minimal changes.
  > >
  > > Fine by me,

  > I've pushed this, together with the choosing logic suggested previously. 
  > Apparently it's the best we can do.

Could you describe the new behavior that you have implemented?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! See stallman.org/skype.html.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Wed, 22 Apr 2015 04:47:44 -0400
> From: Richard Stallman <rms@gnu.org>
> CC: 20292@debbugs.gnu.org, dgutov@yandex.ru, eliz@gnu.org
> 
>   > >> Then my vote is for using "git add FILE" with conflicts during a
>   > >> merge, and "git reset HEAD FILE" with conflicts during "stash pop".
>   > >> I think this is the simplest solution and is easy to implement with
>   > >> minimal changes.
>   > >
>   > > Fine by me,
> 
>   > I've pushed this, together with the choosing logic suggested previously. 
>   > Apparently it's the best we can do.
> 
> Could you describe the new behavior that you have implemented?

The new behavior changes what Emacs does when you save a file which
had conflicts that you resolved:

  . if the conflicts were due to a merge (which includes the automatic
    merge done by "git pull"), the behavior is as before: Emacs stages
    the file for commit by running "git add FILE"

  . if the conflicts were due to something else (which includes
    conflicts during "git stash pop"; not sure if there are other
    non-merge situations that create conflicts), then the new behavior
    is to run "git reset FILE", which leaves any changes in FILE
    uncommitted (and not staged), thus restoring the status of FILE
    before "git stash save"

The one thing to remember is that after saving the file whose
conflicts were resolved, you should type "C-x v v" to commit it in the
first case, and do nothing in the second.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It seems good.

  > The one thing to remember is that after saving the file whose
  > conflicts were resolved, you should type "C-x v v" to commit it in the
  > first case, and do nothing in the second.

Does VC determine automatically whether the conflict was due to merge
or due to git stash pop,
or do you tell VC by doing C-x v v in the first case and nothing
in the second case?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! See stallman.org/skype.html.

bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file

[Eli Zaretskii]

> Date: Wed, 22 Apr 2015 15:59:46 -0400
> From: Richard Stallman <rms@gnu.org>
> CC: dgutov@yandex.ru, 20292@debbugs.gnu.org, dgutov@yandex.ru
> 
>   > The one thing to remember is that after saving the file whose
>   > conflicts were resolved, you should type "C-x v v" to commit it in the
>   > first case, and do nothing in the second.
> 
> Does VC determine automatically whether the conflict was due to merge
> or due to git stash pop,
> or do you tell VC by doing C-x v v in the first case and nothing
> in the second case?

It determines automatically.  It has to, because what it does when you
save the file already depends on that.  When you type "C-x v v" after
saving it, it's too late.