[PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[Thomas Rast]

The wording seems to suggest that you can only globally disable rerere
with the rerere.enabled setting.  But the code actually consults the
config first, and even creates rr-cache for the user if needed.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Noticed while discussing rerere on IRC.  "Can be disabled by setting
this option to false" is of course correct, but insinuates that
setting it to 'true' still falls back to rr-cache, which it doesn't.

git-rerere(1) does not mention the rr-cache fallback; I decided not to
touch it as it's a bit of an implementation detail.  OTOH the
auto-creation of rr-cache can cause strange behavior if a user has
rerere.enabled unset and tries it once, as in

  git config rerere.enabled true
  git merge ...
  git config --unset rerere.enabled


 Documentation/config.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 68cf702..04f5e19 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1783,10 +1783,10 @@ rerere.autoupdate::
 
 rerere.enabled::
 	Activate recording of resolved conflicts, so that identical
-	conflict hunks can be resolved automatically, should they
-	be encountered again.  linkgit:git-rerere[1] command is by
-	default enabled if you create `rr-cache` directory under
-	`$GIT_DIR`, but can be disabled by setting this option to false.
+	conflict hunks can be resolved automatically, should they be
+	encountered again.  By default, linkgit:git-rerere[1] is
+	enabled if there is an `rr-cache` directory under the
+	`$GIT_DIR`.
 
 sendemail.identity::
 	A configuration identity. When given, causes values in the
-- 
1.7.8.2.479.g7c686

Re: [PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> git-rerere(1) does not mention the rr-cache fallback; I decided not to
> touch it as it's a bit of an implementation detail.

It is not an implementation detail but is a (n old) part of the external
UI. Creating .git/rr-cache directory has been the only way to enable it
for quite a while (ever since it was first written as a Perl script early
2006, and the design even survived the rewrite in C late 2006). When the
configuration variable rerere.enabled was introduced with b4372ef (Enable
"git rerere" by the config variable rerere.enabled, 2007-07-06), we
deliberately kept the external interface compatible to avoid disruption.

And your "By default, ... is enabled if there is ... directory" below is
exactly the right description.

The manual page for "rerere" talks about "configuration variable
rerere.enabled"; perhaps it should also refer to git config manual page to
make it more discoverable?

> ... OTOH the
> auto-creation of rr-cache can cause strange behavior if a user has
> rerere.enabled unset and tries it once, as in
>
>   git config rerere.enabled true
>   git merge ...
>   git config --unset rerere.enabled

That is because the last one should be

	git config --bool rerere.enabled false

Perhaps the description for "--unset" option in the manual of "git config"
is not clear enough that there is a difference between a variable not
being set (i.e. we do not know anything about what the user wants) and a
variable explicitly set to false (i.e. we do know the user does not want
it)?  I doubt it, but you may want to check and clarify the section if
needed.

The patch itself looks good; it goes in the right direction.

Thanks.

>  Documentation/config.txt |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 68cf702..04f5e19 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1783,10 +1783,10 @@ rerere.autoupdate::
>  
>  rerere.enabled::
>  	Activate recording of resolved conflicts, so that identical
> -	conflict hunks can be resolved automatically, should they
> -	be encountered again.  linkgit:git-rerere[1] command is by
> -	default enabled if you create `rr-cache` directory under
> -	`$GIT_DIR`, but can be disabled by setting this option to false.
> +	conflict hunks can be resolved automatically, should they be
> +	encountered again.  By default, linkgit:git-rerere[1] is
> +	enabled if there is an `rr-cache` directory under the
> +	`$GIT_DIR`.
>  
>  sendemail.identity::
>  	A configuration identity. When given, causes values in the

Re: [PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[Thomas Rast]

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

> The manual page for "rerere" talks about "configuration variable
> rerere.enabled"; perhaps it should also refer to git config manual page to
> make it more discoverable?

Maybe, but it already says you should set the variable in two different
places.

> Thomas Rast <trast@student.ethz.ch> writes:
>
>> ... OTOH the
>> auto-creation of rr-cache can cause strange behavior if a user has
>> rerere.enabled unset and tries it once, as in
>>
>>   git config rerere.enabled true
>>   git merge ...
>>   git config --unset rerere.enabled
>
> That is because the last one should be
>
> 	git config --bool rerere.enabled false

I definitely meant --unset.  If the user knows the distinction, and
wants to return the variable to the state it had before his test
(perhaps so that a future --global setting might take effect), he would
use this sequence.  He might then be somewhat surprised to see that
rerere is now permamently enabled for this repo.

Probably I'm worrying too much about a weird fringe case though.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> The manual page for "rerere" talks about "configuration variable
>> rerere.enabled"; perhaps it should also refer to git config manual page to
>> make it more discoverable?
>
> Maybe, but it already says you should set the variable in two different
> places.

That is not the point.

The documentation for git config seems to be the only place where we
explain that the existence of rr-cache determines what happens when the
user does _not_ set the variable; lack of that description will lead to
the confusion you describe below:

>> Thomas Rast <trast@student.ethz.ch> writes:
>>
>>> ... OTOH the
>>> auto-creation of rr-cache can cause strange behavior if a user has
>>> rerere.enabled unset and tries it once, as in
>>>
>>>   git config rerere.enabled true
>>>   git merge ...
>>>   git config --unset rerere.enabled
>>
>> That is because the last one should be
>>
>> 	git config --bool rerere.enabled false
>
> I definitely meant --unset.  If the user knows the distinction, and
> wants to return the variable to the state it had before his test ...

Running "unset" is how to return the _variable_ to the previous state, but
that is _not_ how to return to the previous state of the _repository_.

Perhaps something like this in addition?


diff --git a/Documentation/config.txt b/Documentation/config.txt
index 04f5e19..c523c67 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1786,7 +1786,8 @@ rerere.enabled::
 	conflict hunks can be resolved automatically, should they be
 	encountered again.  By default, linkgit:git-rerere[1] is
 	enabled if there is an `rr-cache` directory under the
-	`$GIT_DIR`.
+	`$GIT_DIR`, e.g. if "rerere" was previously used in the
+	repository.
 
 sendemail.identity::
 	A configuration identity. When given, causes values in the

Re: [PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]

[Thomas Rast]

Sorry I'm late, but...

On Fri, 6 Jan 2012 21:17:12 -0800, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Running "unset" is how to return the _variable_ to the previous state, but
> that is _not_ how to return to the previous state of the _repository_.
> 
> Perhaps something like this in addition?
[...]
>  	encountered again.  By default, linkgit:git-rerere[1] is
>  	enabled if there is an `rr-cache` directory under the
> -	`$GIT_DIR`.
> +	`$GIT_DIR`, e.g. if "rerere" was previously used in the
> +	repository.

I don't feel strongly about it, but this is a really neat and concise
way to put it, so why not.  Here's hoping I can save you some work:

---- 8< ---- 8< ----
From: Junio C Hamano <gitster@pobox.com>
Subject: Documentation: rerere's rr-cache auto-creation and rerere.enabled

The description of rerere.enabled left the user in the dark as to who
might create an rr-cache directory.  Add a note that simply invoking
rerere does this.

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 04f5e19..c523c67 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1786,7 +1786,8 @@ rerere.enabled::
 	conflict hunks can be resolved automatically, should they be
 	encountered again.  By default, linkgit:git-rerere[1] is
 	enabled if there is an `rr-cache` directory under the
-	`$GIT_DIR`.
+	`$GIT_DIR`, e.g. if "rerere" was previously used in the
+	repository.
 
 sendemail.identity::
 	A configuration identity. When given, causes values in the


-- 
Thomas Rast
trast@{inf,student}.ethz.ch