[RFC] improving advice message from "git commit" during a merge

[RFC] improving advice message from "git commit" during a merge

[Junio C Hamano]

When there are unmerged paths, you would often get something like
this:

    [git.git (pu|MERGING]$ git commit
    U       copy.c
    U       wrapper.c
    error: commit is not possible because you have unmerged files.
    hint: Fix them up in the work tree, and then use 'git add/rm <file>'
    hint: as appropriate to mark resolution and make a commit, or use
    hint: 'git commit -a'.
    fatal: Exiting because of an unresolved conflict.

which is all good and correct, but I am wondering if we can be a bit
more helpful by customizing the message in various ways.

 - When all the unmerged paths have their conflicts resolved in the
   working tree, we do not have to say "Fix them up in the work
   tree,".  We can instead say "You seem to have fixed them up in
   the work tree already," or something.

 - When some of the unmerged paths have their conflicts still in the
   working tree, we can name them separately from the ones that have
   already been dealt with.

    U       copy.c
    U       wrapper.c (conflicts already resolved in the working tree)

 - Hasty-and-careless new users will be incorrectly enticed to type
   the command given by "or use 'git commit -a'" at the end of this
   advice message without thinking.  Perhaps it is safer to stop the
   sentence at "... and make a commit." and drop that last bit while
   there are conflicts still in the working tree files.  We should
   use the current end-of-message only when all the conflicts have
   been resolved in the working tree.

 - The "/rm" in "use 'git add/rm <file>'" is often useless, as it is
   much rarer to remove a path than adding one.  Perhaps show that
   part only when there is a conflicted path with stage #2 but not
   stage #3 (i.e. they kept what we removed) or vice versa.  "add"
   needs to stay there no matter what, as that is how we tell the
   index "this is the final content".

I am not doing this myself soon, though.  Hint, hint...

Re: [RFC] improving advice message from "git commit" during a merge

[Jeff King]

On Wed, Aug 27, 2014 at 11:23:08AM -0700, Junio C Hamano wrote:

> When there are unmerged paths, you would often get something like
> this:
> 
>     [git.git (pu|MERGING]$ git commit
>     U       copy.c
>     U       wrapper.c
>     error: commit is not possible because you have unmerged files.
>     hint: Fix them up in the work tree, and then use 'git add/rm <file>'
>     hint: as appropriate to mark resolution and make a commit, or use
>     hint: 'git commit -a'.
>     fatal: Exiting because of an unresolved conflict.
> 
> which is all good and correct, but I am wondering if we can be a bit
> more helpful by customizing the message in various ways.
> 
>  - When all the unmerged paths have their conflicts resolved in the
>    working tree, we do not have to say "Fix them up in the work
>    tree,".  We can instead say "You seem to have fixed them up in
>    the work tree already," or something.

How are you determining what has been resolved? By looking for "<<<<<<<"
markers? That feels a little flaky, but I guess it would probably work
well enough in practice.

If we started using that heuristic, it would probably make sense to
teach "git status" about it (and then maybe just have a failed "commit"
rely on wt_status to produce the output).

> I am not doing this myself soon, though.  Hint, hint...

Me either, though it all seems like a sensible direction to me.

-Peff

Re: [RFC] improving advice message from "git commit" during a merge

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>>  - When all the unmerged paths have their conflicts resolved in the
>>    working tree, we do not have to say "Fix them up in the work
>>    tree,".  We can instead say "You seem to have fixed them up in
>>    the work tree already," or something.
>
> How are you determining what has been resolved? By looking for "<<<<<<<"
> markers? That feels a little flaky, but I guess it would probably work
> well enough in practice.

I was going to say "whatever rerere does" by reusing whatever it has
already implemented.

> If we started using that heuristic, it would probably make sense to
> teach "git status" about it (and then maybe just have a failed "commit"
> rely on wt_status to produce the output).
>
>> I am not doing this myself soon, though.  Hint, hint...
>
> Me either, though it all seems like a sensible direction to me.

[PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Matthieu Moy]

'git commit -a' is rarely a good way to mark conflicts as resolved: the
user anyway has to go manually through the list of conflicts to do the
actual resolution, and it is usually better to use "git add" on each
files after doing the resolution.

On the other hand, using 'git commit -a' is potentially dangerous, as it
makes it very easy to mistakenly commit conflict markers without
noticing.

While we're there, synchronize the 'git pull' and 'git merge' messages:
the first was ending with '...  and make a commit.', but not the later.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
>  - Hasty-and-careless new users will be incorrectly enticed to type
>    the command given by "or use 'git commit -a'" at the end of this
>    advice message without thinking.  Perhaps it is safer to stop the
>    sentence at "... and make a commit." and drop that last bit while
>    there are conflicts still in the working tree files.  We should
>    use the current end-of-message only when all the conflicts have
>    been resolved in the working tree.

It was already on my todo-list, as a friend of mine semi-beginner with
Git complained about the mis-advice the other day, and I had to agree.

Eventually, git could detect that conflicts have been resolved, but
then that would be a different message, as not only "use git commit
-a" could be resurected, but "Fix them up in the work tree" should be
dropped when it is the case.

> I am not doing this myself soon, though.  Hint, hint...

I guess I'm just taking the low hanging fruit here ;-).

 advice.c    | 3 +--
 git-pull.sh | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/advice.c b/advice.c
index 9b42033..3b8bf3c 100644
--- a/advice.c
+++ b/advice.c
@@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me)
 		 * other commands doing a merge do.
 		 */
 		advise(_("Fix them up in the work tree, and then use 'git add/rm <file>'\n"
-			 "as appropriate to mark resolution and make a commit, or use\n"
-			 "'git commit -a'."));
+			 "as appropriate to mark resolution and make a commit."));
 	return -1;
 }
 
diff --git a/git-pull.sh b/git-pull.sh
index 18a394f..4d4fc77 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -20,7 +20,7 @@ die_conflict () {
     if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
 	die "$(gettext "Pull is not possible because you have unmerged files.
 Please, fix them up in the work tree, and then use 'git add/rm <file>'
-as appropriate to mark resolution, or use 'git commit -a'.")"
+as appropriate to mark resolution and make a commit.")"
     else
 	die "$(gettext "Pull is not possible because you have unmerged files.")"
     fi
-- 
2.0.2.737.gfb43bde

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> 'git commit -a' is rarely a good way to mark conflicts as resolved: the
> user anyway has to go manually through the list of conflicts to do the
> actual resolution, and it is usually better to use "git add" on each
> files after doing the resolution.
>
> On the other hand, using 'git commit -a' is potentially dangerous, as it
> makes it very easy to mistakenly commit conflict markers without
> noticing.
>
> While we're there, synchronize the 'git pull' and 'git merge' messages:
> the first was ending with '...  and make a commit.', but not the later.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>>  - Hasty-and-careless new users will be incorrectly enticed to type
>>    the command given by "or use 'git commit -a'" at the end of this
>>    advice message without thinking.  Perhaps it is safer to stop the
>>    sentence at "... and make a commit." and drop that last bit while
>>    there are conflicts still in the working tree files.  We should
>>    use the current end-of-message only when all the conflicts have
>>    been resolved in the working tree.
>
> It was already on my todo-list, as a friend of mine semi-beginner with
> Git complained about the mis-advice the other day, and I had to agree.



> Eventually, git could detect that conflicts have been resolved, but
> then that would be a different message, as not only "use git commit
> -a" could be resurected, but "Fix them up in the work tree" should be
> dropped when it is the case.

This paragraph should be in the log message, shouldn't it, probably
with s/could/should/?

> I guess I'm just taking the low hanging fruit here ;-).

I'd say it is more like scooping a fruit lying on the ground before
it rots, but thanks anyway ;-)

>  advice.c    | 3 +--
>  git-pull.sh | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/advice.c b/advice.c
> index 9b42033..3b8bf3c 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me)
>  		 * other commands doing a merge do.
>  		 */
>  		advise(_("Fix them up in the work tree, and then use 'git add/rm <file>'\n"
> -			 "as appropriate to mark resolution and make a commit, or use\n"
> -			 "'git commit -a'."));
> +			 "as appropriate to mark resolution and make a commit."));
>  	return -1;
>  }
>  
> diff --git a/git-pull.sh b/git-pull.sh
> index 18a394f..4d4fc77 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -20,7 +20,7 @@ die_conflict () {
>      if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
>  	die "$(gettext "Pull is not possible because you have unmerged files.
>  Please, fix them up in the work tree, and then use 'git add/rm <file>'
> -as appropriate to mark resolution, or use 'git commit -a'.")"
> +as appropriate to mark resolution and make a commit.")"
>      else
>  	die "$(gettext "Pull is not possible because you have unmerged files.")"
>      fi

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> 'git commit -a' is rarely a good way to mark conflicts as resolved: the
>> user anyway has to go manually through the list of conflicts to do the
>> actual resolution, and it is usually better to use "git add" on each
>> files after doing the resolution.
>>
>> On the other hand, using 'git commit -a' is potentially dangerous, as it
>> makes it very easy to mistakenly commit conflict markers without
>> noticing.
>>
>> While we're there, synchronize the 'git pull' and 'git merge' messages:
>> the first was ending with '...  and make a commit.', but not the later.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>>>  - Hasty-and-careless new users will be incorrectly enticed to type
>>>    the command given by "or use 'git commit -a'" at the end of this
>>>    advice message without thinking.  Perhaps it is safer to stop the
>>>    sentence at "... and make a commit." and drop that last bit while
>>>    there are conflicts still in the working tree files.  We should
>>>    use the current end-of-message only when all the conflicts have
>>>    been resolved in the working tree.
>>
>> It was already on my todo-list, as a friend of mine semi-beginner with
>> Git complained about the mis-advice the other day, and I had to agree.
>
>
>
>> Eventually, git could detect that conflicts have been resolved, but
>> then that would be a different message, as not only "use git commit
>> -a" could be resurected, but "Fix them up in the work tree" should be
>> dropped when it is the case.
>
> This paragraph should be in the log message, shouldn't it, probably
> with s/could/should/?

I think the commit message explains well enough why the change is good.
The additional paragraph explains why I did not do it the way your
message suggests (which has to do with the current discussion, but does
not have to be recorded in history IHMO).

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

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Jonathan Nieder]

Matthieu Moy wrote:

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
[...]
> ---
>  advice.c    | 3 +--
>  git-pull.sh | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)

Thanks for taking it on.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[...]
> It was already on my todo-list, as a friend of mine semi-beginner with
> Git complained about the mis-advice the other day, and I had to agree.

That's a useful sort of thing to put in a commit message. :)

> Eventually, git could detect that conflicts have been resolved, but
> then that would be a different message, as not only "use git commit
> -a" could be resurected, but "Fix them up in the work tree" should be
> dropped when it is the case.

As is this --- when I wonder why code isn't a certain way, ideas for
future work found in the description for the blamed commit are often
helpful in explaining the current state and saving me from blind
alleys in changing it.

Anyway, this is already a very good change as-is.

Actually, I'd be nervous about suggesting "use git commit -a" without
at least also saying "inspect the result or run tests" in the
no-conflict-markers-found case.  Rerere sometimes makes mistakes, and
the result of picking one side when merging binary files can be even
worse.

Thanks,
Jonathan

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

>> It was already on my todo-list, as a friend of mine semi-beginner with
>> Git complained about the mis-advice the other day, and I had to agree.
>
> That's a useful sort of thing to put in a commit message. :)
>
>> Eventually, git could detect that conflicts have been resolved, but
>> then that would be a different message, as not only "use git commit
>> -a" could be resurected, but "Fix them up in the work tree" should be
>> dropped when it is the case.
>
> As is this --- when I wonder why code isn't a certain way, ideas for
> future work found in the description for the blamed commit are often
> helpful in explaining the current state and saving me from blind
> alleys in changing it.

Yes.

> Anyway, this is already a very good change as-is.
>
> Actually, I'd be nervous about suggesting "use git commit -a" without
> at least also saying "inspect the result or run tests" in the
> no-conflict-markers-found case.  Rerere sometimes makes mistakes, and
> the result of picking one side when merging binary files can be even
> worse.

Here is how I phrased in the one queued tentatively.

-- >8 --
From: Matthieu Moy <Matthieu.Moy@imag.fr>
Date: Thu, 28 Aug 2014 11:46:58 +0200
Subject: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

'git commit -a' is rarely a good way to mark conflicts as resolved:
the user anyway has to go manually through the list of conflicts to
do the actual resolution, and it is usually better to use "git add"
on each files after doing the resolution.

On the other hand, using 'git commit -a' is potentially dangerous,
as it makes it very easy to mistakenly commit conflict markers
without noticing, and even worse, the user may have started a merge
while having local changes that do not overlap with it in the
working tree.

While we're there, synchronize the 'git pull' and 'git merge'
messages: the first was ending with '...  and make a commit.', but
not the latter.

Eventually, git should detect that conflicts have been resolved in
the working tree and tailor these messages further.  Not only "use
git commit -a" could be resurected, but "Fix them up in the work
tree" should be dropped when it happens.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/advice.c b/advice.c
index 9b42033..3b8bf3c 100644
--- a/advice.c
+++ b/advice.c
@@ -86,8 +86,7 @@ int error_resolve_conflict(const char *me)
 		 * other commands doing a merge do.
 		 */
 		advise(_("Fix them up in the work tree, and then use 'git add/rm <file>'\n"
-			 "as appropriate to mark resolution and make a commit, or use\n"
-			 "'git commit -a'."));
+			 "as appropriate to mark resolution and make a commit."));
 	return -1;
 }
 
diff --git a/git-pull.sh b/git-pull.sh
index 18a394f..4d4fc77 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -20,7 +20,7 @@ die_conflict () {
     if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
 	die "$(gettext "Pull is not possible because you have unmerged files.
 Please, fix them up in the work tree, and then use 'git add/rm <file>'
-as appropriate to mark resolution, or use 'git commit -a'.")"
+as appropriate to mark resolution and make a commit.")"
     else
 	die "$(gettext "Pull is not possible because you have unmerged files.")"
     fi

Re: [PATCH] merge, pull: stop advising 'commit -a' in case of conflict

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Here is how I phrased in the one queued tentatively.

OK with me, thanks,

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

Re: [RFC] improving advice message from "git commit" during a merge

[Stefan Beller]

On 27.08.2014 20:23, Junio C Hamano wrote:
> I am not doing this myself soon, though.  Hint, hint...

I asked once upon a time, if there was a place, 
which collects such topics for casual contributors and new comers.

Would you mind to update the leftover bits at
http://git-blame.blogspot.de/search?q=leftover&by-date=true
?

Thanks,
Stefan

Re: [RFC] improving advice message from "git commit" during a merge

[Junio C Hamano]

Stefan Beller <stefanbeller@gmail.com> writes:

> On 27.08.2014 20:23, Junio C Hamano wrote:
>> I am not doing this myself soon, though.  Hint, hint...
>
> I asked once upon a time, if there was a place, 
> which collects such topics for casual contributors and new comers.
>
> Would you mind to update the leftover bits at
> http://git-blame.blogspot.de/search?q=leftover&by-date=true
> ?

They live in a bit more permanent home at

  http://git-blame.blogspot.com/p/leftover-bits.html

these days.

It is too early to tell this particular one will make an entry there
yet ;-)