[PATCH] Documentation/git-rerere: document 'remaining' command

[PATCH] Documentation/git-rerere: document 'remaining' command

[Vincent van Ravesteijn]

From: Vincent van Ravesteijn <vfr@lyx.org>

This adds the 'remaining' command to the documentation of
'git rerere'. This command was added in ac49f5ca (Feb 16 2011;
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>) but
it was never documented.

Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
 Documentation/git-rerere.txt |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index a6253ba..b75d34b 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -8,7 +8,7 @@ git-rerere - Reuse recorded resolution of conflicted merges
 SYNOPSIS
 --------
 [verse]
-'git rerere' ['clear'|'forget' <pathspec>|'diff'|'status'|'gc']
+'git rerere' ['clear'|'forget' <pathspec>|'diff'|'remaining'|'status'|'gc']
 
 DESCRIPTION
 -----------
@@ -53,6 +53,14 @@ useful for tracking what has changed while the user is resolving
 conflicts.  Additional arguments are passed directly to the system
 'diff' command installed in PATH.
 
+'remaining'::
+
+Like 'diff', but this only prints the unresolved filenames. This
+includes the files for which rerere tracks the resolution (as shown
+by 'git rerere status'), but also the files for which the conflicts
+cannot be tracked by rerere, e.g. files with conflicts that do not 
+have both our side and their side like "we modified, they deleted".
+
 'status'::
 
 Like 'diff', but this only prints the filenames that will be tracked
-- 
1.7.5.4

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Junio C Hamano]

Vincent van Ravesteijn <vfr@lyx.org> writes:

> From: Vincent van Ravesteijn <vfr@lyx.org>
>
> This adds the 'remaining' command to the documentation of
> 'git rerere'. This command was added in ac49f5ca (Feb 16 2011;
> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>) but
> it was never documented.
>
> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
> ---
>  Documentation/git-rerere.txt |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
> index a6253ba..b75d34b 100644
> --- a/Documentation/git-rerere.txt
> +++ b/Documentation/git-rerere.txt
> @@ -8,7 +8,7 @@ git-rerere - Reuse recorded resolution of conflicted merges
>  SYNOPSIS
>  --------
>  [verse]
> -'git rerere' ['clear'|'forget' <pathspec>|'diff'|'status'|'gc']
> +'git rerere' ['clear'|'forget' <pathspec>|'diff'|'remaining'|'status'|'gc']
>  
>  DESCRIPTION
>  -----------
> @@ -53,6 +53,14 @@ useful for tracking what has changed while the user is resolving
>  conflicts.  Additional arguments are passed directly to the system
>  'diff' command installed in PATH.
>  
> +'remaining'::
> +
> +Like 'diff', but this only prints the unresolved filenames. This

What aspect of 'rerere remaining' is like 'rerere diff'?

In general, people should think twice after writing "Like X, but Y"
and try to come up with a more clear description that does not have
to force the user to read about X and then selectively forget what
he just read and replace it with Y. This is especially true when the
Y part is significantly different from what X does.

Another phrase to watch out for when writing documentation is "X. In
other words, Y."  People do this after writing X and finding it hard
to understand and necessary to explain in easier terms.  Reading it
again without "X. In other words, " often yields a better description.

	'remaining'::

        Print paths with conflicts that are not resolved.

Should be sufficient, I think.

In fact, wouldn't this be more or less equivalent to "ls-files -u"
without anything other than name part?

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Phil Hord]

On Tue, Mar 6, 2012 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Vincent van Ravesteijn <vfr@lyx.org> writes:
>
>> From: Vincent van Ravesteijn <vfr@lyx.org>
>>
>> This adds the 'remaining' command to the documentation of
>> 'git rerere'. This command was added in ac49f5ca (Feb 16 2011;
>> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>) but
>> it was never documented.
>>
>> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
>> ---
>>  Documentation/git-rerere.txt |   10 +++++++++-
>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
>> index a6253ba..b75d34b 100644
>> --- a/Documentation/git-rerere.txt
>> +++ b/Documentation/git-rerere.txt
>> @@ -8,7 +8,7 @@ git-rerere - Reuse recorded resolution of conflicted merges
>>  SYNOPSIS
>>  --------
>>  [verse]
>> -'git rerere' ['clear'|'forget' <pathspec>|'diff'|'status'|'gc']
>> +'git rerere' ['clear'|'forget' <pathspec>|'diff'|'remaining'|'status'|'gc']
>>
>>  DESCRIPTION
>>  -----------
>> @@ -53,6 +53,14 @@ useful for tracking what has changed while the user is resolving
>>  conflicts.  Additional arguments are passed directly to the system
>>  'diff' command installed in PATH.
>>
>> +'remaining'::
>> +
>> +Like 'diff', but this only prints the unresolved filenames. This
>
[...]
>
>        'remaining'::
>
>        Print paths with conflicts that are not resolved.
>
> Should be sufficient, I think.
>
> In fact, wouldn't this be more or less equivalent to "ls-files -u"
> without anything other than name part?

No.  When using  --no-rerere-autoupdate, git does not add autoresolved
files to the index; it fixes them only in your working directory.
'ls-files -u' still lists them as unresolved.  'rerere remaining' does
not list these autoresolved files.  'mergetool' uses this command to
avoid asking the user to resolve files which git rerere already
resolved for her.

ac49f5ca8 has a pretty complete description, though it may be a bit
too wordy for the the up-front synopsis.
2f59c9470 has a more complete justification.

Phil

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

> On Tue, Mar 6, 2012 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>>        'remaining'::
>>
>>        Print paths with conflicts that are not resolved.
>>
>> Should be sufficient, I think.
>
> ....  'mergetool' uses this command to
> avoid asking the user to resolve files which git rerere already
> resolved for her.

Ok, so "Print paths with conflicts that are not resolved." indeed is
sufficient.

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Phil Hord]

On Wed, Mar 7, 2012 at 5:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Phil Hord <phil.hord@gmail.com> writes:
>
>> On Tue, Mar 6, 2012 at 2:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>>        'remaining'::
>>>
>>>        Print paths with conflicts that are not resolved.
>>>
>>> Should be sufficient, I think.
>>
>> ....  'mergetool' uses this command to
>> avoid asking the user to resolve files which git rerere already
>> resolved for her.
>
> Ok, so "Print paths with conflicts that are not resolved." indeed is
> sufficient.

If you goal is to say as little as possible, then yes.  But I had to
read the related commit messages several times before it dawned on me
what the distinction was.  The main problem was that I didn't
understand that I was missing 'rerere.autoupdate=true' in my config,
or why it mattered.  I only know that rerere was letting me down
sometimes, and 'rerere remaining' seemed to be missing some
clearly-still-unresolved files.

Thanks to this proposal, I understand it better now.  But not from
reading this email thread.

Phil

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

>>> .... 'mergetool' uses this command to
>>> avoid asking the user to resolve files which git rerere already
>>> resolved for her.
>>
>> Ok, so "Print paths with conflicts that are not resolved." indeed is
>> sufficient.
>
> If you goal is to say as little as possible, then yes.  But I had to
> read the related commit messages several times before it dawned on me
> what the distinction was.

The goal is "Concise, coherent and clear"; "as little as possible"
never is.  We need to elaborate as needed but make sure we do not
tire readers with irrelevant explanation.

The first problem I had with the patch (go back and re-read the
patch and its initial review) was "Like 'diff', but...".  It is not
"Like diff" at all (if anything, it is more like "status", but
"status" in turn is not "Like diff" either). We can first drop that
part and spend more words to describe what it really is.

> The main problem was that I didn't
> understand that I was missing 'rerere.autoupdate=true' in my config,
> or why it mattered. I only know that rerere was letting me down
> sometimes, and 'rerere remaining' seemed to be missing some
> clearly-still-unresolved files.

Personally, I think you are being *good* by not using autoupdate.
Once you let rerere auto-update, it will become hard to notice a
mismerge when previous resolution is applied when it shouldn't, and
even harder to correct it ("checkout -m" will not work).

> Thanks to this proposal, I understand it better now.  But not from
> reading this email thread.

Care to give a crack at it, then?

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Phil Hord]

On Thu, Mar 8, 2012 at 12:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Phil Hord <phil.hord@gmail.com> writes:
>
>>>> .... 'mergetool' uses this command to
>>>> avoid asking the user to resolve files which git rerere already
>>>> resolved for her.
>>>
>>> Ok, so "Print paths with conflicts that are not resolved." indeed is
>>> sufficient.
>>
>> If you goal is to say as little as possible, then yes.  But I had to
>> read the related commit messages several times before it dawned on me
>> what the distinction was.
>
> The goal is "Concise, coherent and clear"; "as little as possible"
> never is.  We need to elaborate as needed but make sure we do not
> tire readers with irrelevant explanation.
>
> The first problem I had with the patch (go back and re-read the
> patch and its initial review) was "Like 'diff', but...".  It is not
> "Like diff" at all (if anything, it is more like "status", but
> "status" in turn is not "Like diff" either). We can first drop that
> part and spend more words to describe what it really is.

No disagreement here.

>> The main problem was that I didn't
>> understand that I was missing 'rerere.autoupdate=true' in my config,
>> or why it mattered. I only know that rerere was letting me down
>> sometimes, and 'rerere remaining' seemed to be missing some
>> clearly-still-unresolved files.
>
> Personally, I think you are being *good* by not using autoupdate.
> Once you let rerere auto-update, it will become hard to notice a
> mismerge when previous resolution is applied when it shouldn't, and
> even harder to correct it ("checkout -m" will not work).

Slightly off-topic, but here's why autoupdate=false is a problem, and
why I thought rerere-remaining was broken for the last year.  When I
rebase a branch, git magically plows through dozens of commits and
stops to tell me when there's a merge conflict that needs my
attention.  Not having a GUI means I have to do something (type
commands or parse output) to see what's happened.  But the solution to
"resolve these conflicts" is always the same:  git-mergetool && git
rebase --continue.

Except with rerere enabled.

  $ git rebase origin/master
   Noise, noise, noise
   Resolved 'foo' using previous resolution.
   Failed to merge in the changes.

I reach for my utility belt, but my usual tricks all fail at this point:

   $ git mergetool
   No files need merging

   $ git rebase --continue
   foo: needs merge

   $ git diff foo
   diff --cc foo
   index 5716ca5,7601807..0000000
   --- i/foo
   +++ w/foo

The last one looks to me, the uninitiated, like a file with no
changes. I know better now, but I didn't before.  The only thing that
works is this, but I didn't know why:

   $ git add foo

Now I know why, and it just pisses me off.  I thought rerere was
supposed to save me from this drudgery, especially when I am rebasing
the same branch multiple times (part of a migration effort).

I think I know the solution now.  'mergetool' should never have
ignored these files.  Instead, it should tell me about them:

   $ git mergetool
   Merging:
   foo

   Normal merge conflict for 'foo' was resolved using previous resolution.
   hint: to re-examine the resolution, use "git diff -c foo"
   hint: to accept this resolution, add it to the index with "git add foo"

   No more files need merging.

I didn't know how to spell "diff -c" before, but I also didn't know it
was what I needed

>> Thanks to this proposal, I understand it better now.  But not from
>> reading this email thread.
>
> Care to give a crack at it, then?

Yeah, right.  Remember, it took me three tries reading the actual
commit messages to understand what was even going on.  I'm probably
too thick for this.

How's this:

-- >8 --
Subject: [PATCH] rerere: Document 'rerere remaining'

This adds the 'remaining' command to the documentation of
'git rerere'. This command was added in ac49f5ca (Feb 16 2011;
Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>) but
it was never documented.

Touch up the other rerere commands to reduce noise.

Signed-off-by: Phil Hord <phil.hord@gmail.com>
Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>
---
Do you think the touch-ups are overkill?
Also, I think 'rerere diff' could use a rewrite.

 Documentation/git-rerere.txt |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt
index a6253ba..b43b7c8 100644
--- a/Documentation/git-rerere.txt
+++ b/Documentation/git-rerere.txt
@@ -8,7 +8,7 @@ git-rerere - Reuse recorded resolution of conflicted merges
 SYNOPSIS
 --------
 [verse]
-'git rerere' ['clear'|'forget' <pathspec>|'diff'|'status'|'gc']
+'git rerere' ['clear'|'forget' <pathspec>|'diff'|'remaining'|'status'|'gc']

 DESCRIPTION
 -----------
@@ -37,30 +37,35 @@ its working state.

 'clear'::

-This resets the metadata used by rerere if a merge resolution is to be
+Reset the metadata used by rerere if a merge resolution is to be
 aborted.  Calling 'git am [--skip|--abort]' or 'git rebase [--skip|--abort]'
 will automatically invoke this command.

 'forget' <pathspec>::

-This resets the conflict resolutions which rerere has recorded for the current
+Reset the conflict resolutions which rerere has recorded for the current
 conflict in <pathspec>.

 'diff'::

-This displays diffs for the current state of the resolution.  It is
+Display diffs for the current state of the resolution.  It is
 useful for tracking what has changed while the user is resolving
 conflicts.  Additional arguments are passed directly to the system
 'diff' command installed in PATH.

 'status'::

-Like 'diff', but this only prints the filenames that will be tracked
-for resolutions.
+Print paths with conflicts whose merge resolution rerere will record.
+
+'remaining'::
+
+Print paths with conflicts that have not been autoresolved by rerere.
+This includes paths whose resolutions cannot be tracked by rerere,
+such as conflicting submodules.

 'gc'::

-This prunes records of conflicted merges that
+Prune records of conflicted merges that
 occurred a long time ago.  By default, unresolved conflicts older
 than 15 days and resolved conflicts older than 60
 days are pruned.  These defaults are controlled via the
-- 
1.7.9.3

Re: [PATCH] Documentation/git-rerere: document 'remaining' command

[Junio C Hamano]

Phil Hord <phil.hord@gmail.com> writes:

> On Thu, Mar 8, 2012 at 12:23 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Phil Hord <phil.hord@gmail.com> writes:
> ...
>   $ git rebase origin/master
>    Noise, noise, noise
>    Resolved 'foo' using previous resolution.
>    Failed to merge in the changes.
>
> I reach for my utility belt, but my usual tricks all fail at this point:
>
>    $ git mergetool
>    No files need merging
>
>    $ git rebase --continue
>    foo: needs merge

Just to make sure we are on the same page (I do not personally use
"mergetool"), the above looks like a bug in mergetool.

Are you reporting a still-unfixed breakage, or is it an anecdote on
how you were frustrated in the past due to a bug that has since been
fixed?

> How's this:

Looks good.

>  'status'::
>
> -Like 'diff', but this only prints the filenames that will be tracked
> -for resolutions.
> +Print paths with conflicts whose merge resolution rerere will record.
> +
> +'remaining'::
> +
> +Print paths with conflicts that have not been autoresolved by rerere.
> +This includes paths whose resolutions cannot be tracked by rerere,
> +such as conflicting submodules.

We might want to add "and a path that was deleted by one branch and
modified by the other branch" at the end here.

Unless an earlier part of the documentation that discusses what kind
of resolutions get recorded and reapplied makes it clear enough that
the reader can easily guess a delete/modify conflict is not handled,
that is.  I _think_ the description at the top makes it clear the
two branches being merged both need to have a file at the path, so
in that sense, singling out delete/modify and mentioning it here
might be redundant, but I am not the target audience (I wrote and
named it rerere after all ;-), so I shouldn't be my own judge.

Thanks.