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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

>> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]