[PATCH] A more sensible error message why 'git rebase --continue' failed

[PATCH] A more sensible error message why 'git rebase --continue' failed

[Evan Phoenix]

The current message is a bit cryptic, indicating that only changes which contained
merge conflicts should be added to the index, but in fact, all outstanding
changes must be.

The change could also incorporate a -i that prompts the user and performs
'git add -u' if they wish, allowing the rebase to properly continue.

Signed-off-by: Evan Phoenix <evan@fallingsnow.net>

diff --git a/git-rebase.sh b/git-rebase.sh
index cbafa14..c67f29a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -135,8 +135,12 @@ do
 	case "$1" in
 	--continue)
 		git diff-files --quiet || {
-			echo "You must edit all merge conflicts and then"
-			echo "mark them as resolved using git add"
+                        echo "Unable to continue rebase:"
+                        echo "  All modified files must be added to index first."
+                        echo "  Use 'git add' to resolve them and add them to the index."
+                        echo
+                        echo "Outstanding files:"
+                        git diff-files --name-status
 			exit 1
 		}
 		if test -d "$dotest"
-- 
1.5.3.rc5.41.gc1c22

Re: [PATCH] A more sensible error message why 'git rebase --continue' failed

[Junio C Hamano]

Evan Phoenix <evan@fallingsnow.net> writes:

> The current message is a bit cryptic, indicating that only
> changes which contained merge conflicts should be added to the
> index, but in fact, all outstanding changes must be.

That might be true, and we might want a better yet still concise
wording for the suggestion.


Your wording, however, would lead a new user to run "git add"
even before resolving the conflicts, and then make him complain
"why doesn't git do that for me", and then again complain "gaah,
I followed the suggestion and ended up committing garbage with
funny <<< === >>> lines in it".

So in that sense, I do not necessarily see _your_ new wording an
improvement.

> @@ -135,8 +135,12 @@ do
>  	case "$1" in
>  	--continue)
>  		git diff-files --quiet || {
> -			echo "You must edit all merge conflicts and then"
> -			echo "mark them as resolved using git add"
> +                        echo "Unable to continue rebase:"
> +                        echo "  All modified files must be added to index first."

Also I see whitespace breackages here --- did you intend to
replace tabs with spaces, and if so why?

Re: [PATCH] A more sensible error message why 'git rebase --continue' failed

[Evan Phoenix]

On Aug 19, 2007, at 11:25 PM, Junio C Hamano wrote:

> Evan Phoenix <evan@fallingsnow.net> writes:
>
>> The current message is a bit cryptic, indicating that only
>> changes which contained merge conflicts should be added to the
>> index, but in fact, all outstanding changes must be.
>
> That might be true, and we might want a better yet still concise
> wording for the suggestion.
>
>
> Your wording, however, would lead a new user to run "git add"
> even before resolving the conflicts, and then make him complain
> "why doesn't git do that for me", and then again complain "gaah,
> I followed the suggestion and ended up committing garbage with
> funny <<< === >>> lines in it".

Hm, a good point.

How about something like

"Please verify all merge conflicts in all modified files, then run
'git add' to resolve them."

Or perhaps the behavior of rebase needs to be altered a bit
here. 'git ls-files -u' can detect there exists outstanding, unmerged
files. If those still exist, it should certainly stop outright. But,
it seems to me that if there are no unmerged files and still just
normal modified ones (git ls-files -m), 'git rebase --continue' should
be able to run 'git add -u' for the user.

Outside that, I do agree that the wording should be more stern about
checking all modified files before running 'git add'.

>
> So in that sense, I do not necessarily see _your_ new wording an
> improvement.
>
>> @@ -135,8 +135,12 @@ do
>>  	case "$1" in
>>  	--continue)
>>  		git diff-files --quiet || {
>> -			echo "You must edit all merge conflicts and then"
>> -			echo "mark them as resolved using git add"
>> +                        echo "Unable to continue rebase:"
>> +                        echo "  All modified files must be added  
>> to index first."
>
> Also I see whitespace breackages here --- did you intend to
> replace tabs with spaces, and if so why?

A mistake on my part. I sent in a new patch eariler but ended up having
mail server troubles. I've already got a new version that keeps the
tabs as they should be. The next version will be proper.

  - Evan Phoenix / evan@fallingsnow.net

Re: [PATCH] A more sensible error message why 'git rebase --continue' failed

[Steven Grimm]

Evan Phoenix wrote:
> How about something like
>
> "Please verify all merge conflicts in all modified files, then run
> 'git add' to resolve them."

Since we're nitpicking language here... "git add" doesn't resolve 
anything. *You* resolve the conflicts (by editing the file and removing 
the conflict markers and editing the code, by running git mergetool, 
etc.) then you run "git add" to tell git that you're done resolving.

If I literally followed the above, I would run "less" on a file, say, 
"Yep, git flagged all the merge conflicts correctly," then run "git add" 
figuring it would pop me into some kind of conflict resolution UI. 
Actually it's worse than that -- the above implies that all modified 
files will have merge conflicts, which is certainly not true in many cases.

How about:

"Please resolve all merge conflicts, then run 'git add' to tell git you've
finished resolving each file."

-Steve

Re: [PATCH] A more sensible error message why 'git rebase --continue' failed

[Johannes Sixt]

Evan Phoenix wrote:
> 
> The current message is a bit cryptic, indicating that only changes which contained
> merge conflicts should be added to the index, but in fact, all outstanding
> changes must be.

I don't think that all files must be added if rebase -m was used: in
this case, non-conflicting changes are already in the index.

-- Hannes