unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dgutov@yandex.ru>
To: Junio C Hamano <gitster@pobox.com>, 20292@debbugs.gnu.org
Subject: bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file
Date: Wed, 16 Nov 2016 02:25:32 +0200	[thread overview]
Message-ID: <fdbe4315-a300-6487-435d-6e45630e2914@yandex.ru> (raw)
In-Reply-To: <xmqqd1il7lor.fsf@gitster.mtv.corp.google.com>

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.





       reply	other threads:[~2016-11-16  0:25 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <xmqqd1il7lor.fsf@gitster.mtv.corp.google.com>
2016-11-16  0:25 ` Dmitry Gutov [this message]
2016-11-16 17:30   ` bug#20292: 24.5; Saving Git-controlled file with merge conflicts after "stash pop" stages the file Eli Zaretskii
2016-11-16 22:45     ` Dmitry Gutov
2016-11-17 15:57       ` Eli Zaretskii
2016-11-17 18:04         ` Dmitry Gutov
2015-04-10 12:55 Eli Zaretskii
2015-04-18 19:16 ` Dmitry Gutov
2015-04-18 19:31   ` Eli Zaretskii
2015-04-18 21:58     ` Dmitry Gutov
2015-04-18 22:06       ` Dmitry Gutov
2015-04-19 14:30       ` Eli Zaretskii
2015-04-19 16:28         ` Dmitry Gutov
2015-04-19 17:06           ` Eli Zaretskii
2015-04-19 17:38             ` Dmitry Gutov
2015-04-19 18:05               ` Eli Zaretskii
2015-04-19 18:11                 ` Dmitry Gutov
2015-04-19 18:25                   ` Eli Zaretskii
2015-04-19 18:30                     ` Dmitry Gutov
2015-04-19 18:38                       ` Eli Zaretskii
2015-04-19 19:27                         ` Dmitry Gutov
2015-04-19 19:33                           ` Eli Zaretskii
2015-04-20  2:41                       ` Stefan Monnier
2015-04-20 14:45                         ` Eli Zaretskii
2015-04-20 19:20                           ` Stefan Monnier
2015-04-20 19:23                             ` Eli Zaretskii
2015-04-21  1:06                               ` Stefan Monnier
2015-04-22  1:50                                 ` Dmitry Gutov
2015-04-22  7:35                                   ` Eli Zaretskii
2015-05-12 23:13                                     ` Dmitry Gutov
2015-05-13 13:04                                       ` Stefan Monnier
2015-05-13 16:20                                         ` Eli Zaretskii
2015-05-13 16:18                                       ` Eli Zaretskii
2015-05-14  1:24                                         ` Dmitry Gutov
2015-05-14 14:53                                           ` Eli Zaretskii
2015-05-14 18:00                                             ` Dmitry Gutov
2015-05-14 18:49                                               ` Eli Zaretskii
2015-05-14  3:49                                         ` Stefan Monnier
2015-05-14 14:53                                           ` Eli Zaretskii
2015-05-14 15:51                                             ` Stefan Monnier
2015-05-14 17:30                                               ` Dmitry Gutov
2015-05-14 18:36                                                 ` Eli Zaretskii
2015-05-14 18:48                                                   ` Dmitry Gutov
2015-05-14 18:52                                                     ` Eli Zaretskii
2015-05-14 19:09                                                       ` Dmitry Gutov
2015-05-14 19:33                                                         ` Eli Zaretskii
2015-05-14 20:24                                                           ` Dmitry Gutov
2015-05-14 20:55                                                             ` Stefan Monnier
2015-05-15  7:14                                                               ` Eli Zaretskii
2015-05-15 18:13                                                                 ` Stefan Monnier
2015-05-15 18:55                                                                   ` Eli Zaretskii
2015-05-15 20:02                                                                     ` Stefan Monnier
2015-05-15 20:19                                                                       ` Eli Zaretskii
2015-05-15 23:52                                                                         ` Stefan Monnier
2015-05-15 23:57                                                                           ` Dmitry Gutov
2015-05-16 13:48                                                                             ` Stefan Monnier
2015-05-15 23:50                                                               ` Dmitry Gutov
2015-05-16  7:15                                                                 ` Eli Zaretskii
2015-05-16  8:03                                                                   ` Dmitry Gutov
2015-05-16  8:55                                                                     ` Eli Zaretskii
2015-05-16 13:15                                                                       ` Dmitry Gutov
2015-05-16 13:53                                                                 ` Stefan Monnier
2015-05-16 14:13                                                                   ` Dmitry Gutov
2015-05-15  7:10                                                             ` Eli Zaretskii
2015-05-15  7:17                                                             ` Eli Zaretskii
2015-04-22  8:47                                   ` Richard Stallman
2015-04-22  9:16                                     ` Eli Zaretskii
2015-04-22 19:59                                       ` Richard Stallman
2015-04-22 21:32                                         ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fdbe4315-a300-6487-435d-6e45630e2914@yandex.ru \
    --to=dgutov@yandex.ru \
    --cc=20292@debbugs.gnu.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).