Re: [RFC/PATCH] commit: make the error message on unmerged entries user-friendly.

Junio C Hamano [off-list ref] writes:

quoted

Two questions:

* Did anyone actually use the 3 "file: unmerged (sha1)" lines?

I don't think anybody does these days, and even if the question were about
the historical practice, I doubt anybody used "git merge-file" using these
blob object names to come up with a resolution.  Besides, if they really
want to, they can ask "ls-files -u" for that information themselves. 

OK, that's what I thought, but 'wanted to be sure.

I imagine you would say
something like this?

  With this patch, the error looks like this:

  $ git commit
  U foo.txt
  fatal: Commit is not possible because you have unmerged files.
  Please, resolve the conflicts listed above, and then mark them
  as resolved with 'git add <filename>', or use 'git commit -a'

Yes, sorry for not making it more explicit.

Do I like it?  Hmmm.

 - "Please, ", especially with the comma, looks superfluous;

I wouldn't call it mandatory, but it doesn't harm, and

$ grep -i -n -e '"Please' *.c
builtin-commit.c:246:		    "Please, resolve the conflicts listed above, and then mark them\n"
builtin-commit.c:698:			"Please supply the message using either -m or -F option.\n");
builtin-help.c:209:			"Please consider using 'man.<tool>.cmd' instead.",
builtin-help.c:221:			"Please consider using 'man.<tool>.path' instead.",
builtin-remote.c:1124:					"Please choose one explicitly with:");
builtin-tag.c:321:			"Please supply the message using either -m or -F option.\n");
merge-recursive.c:1191:			"Please, commit your changes or stash them before you can merge.";
setup.c:241:		warning("Please upgrade Git");

 - Some people consider "please resolve the conflicts" to mean the whole
   process of "fix them up in the work tree, and mark them resolved in the
   index", and others consider it to mean only the first step to "fix them
   up in the work tree".  I happen to be in the former camp, and to me
   ",and then mark them as..., add <filename>'," look superfluous because
   of that.

   I however think it is more helpful to new people who benefit from this
   advice message if we spell out these two steps.

Yes. Someone having "resolved" (i.e. removed markers), and trying to
commit should get a hint on the commands he can use to achieve this
goal.

   Unfortunately, for that purpose, the description of the latter
   "mark them resolved in the index" step you have looks inadequate;
   e.g. sometimes it needs to be done with "git rm <filename>".

Maybe "sometimes", but to me, that's sufficiently rare to be omited
here (I don't think I ever used 'git rm' to resolve a conflict). The
user manual says this:

,----
| Each time you resolve the conflicts in a file and update the index:
| 
| $ git add file.txt
| 
| the different stages of that file will be "collapsed", after which git
| diff will (by default) no longer show diffs for that file.
`----

and I don't think it makes sense to try to be more exhaustive here
than in the user-manual.

The need to give this advice on how to resolve conflicts is shared among
commands like 'git merge', 'git cherry-pick', and 'git status', to name a
few.

Not sure 'status' needs anything more. It already says

# Unmerged paths:
#   (use "git add/rm <file>..." as appropriate to mark resolution)
#
#       both modified:      foo.txt

and the big difference between 'git status' and the others is that
it's legitimate to run it while resolving conflicts, while calling
'git merge' or 'git commit' can be done only by mistake.

It's not serious to eat 3 or 4 lines of the screen to display a
message to tell the user he shouldn't have done this, but it'd be
disturbing to eat more than 1 line for a common case.

I think we should consolidate them all.

Right, although "commit" is definitely the most important (dumb users
don't need "git merge").

 - Decide if we go one-step (i.e. just say "resolve conflicts" and nothing
   else) or two-step (i.e. say "Fix them up in the work tree" and "mark
   them resolved in the index") approach; I am leaning toward the latter.

Meetoo.

 - Decide the exact language used.  I think "Fix them up in the work tree"
   is passably okay, but "Mark them resolved in the index" needs to be
   made more concrete, and "git add <filename>" is not quite it, I am
   afraid.

See above. To me, pointing to "git commit -a" and "git add" is
sufficient.

Pointing to "git commit -a" is also important to me, because Git
newbies may have been told to always use "git commit" with "-a"
(common use-case: "I have to use Git, I know SVN and I don't want to
learn anything new").

 - Omit issuing the advisory message when advice.resolveConflict is set to
   false.

Sensible, yes.

 - Perhaps have a common helper function to do this from the commands that
   need to give the advice.

Probably not, because the advice will be different:

git merge => please resolve and commit before you can merge
git commit => please resolve before you can commit
...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/