[PATCH/RFC v3 1/4] git-am.sh: fix initialization of the threeway variable

[PATCH/RFC v3 1/4] git-am.sh: fix initialization of the threeway variable

[Remi Lespinet]

Initialization for the threeway variable was missing. This caused
a behavior change for command lines like:

	threeway=t git am ...

This commit fixes the bug.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
---
 git-am.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-am.sh b/git-am.sh
index 761befb..c460dd0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -378,6 +378,7 @@ committer_date_is_author_date=
 ignore_date=
 allow_rerere_autoupdate=
 gpg_sign_opt=
+threeway=
 
 if test "$(git config --bool --get am.messageid)" = true
 then
-- 
1.9.1

[PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Remi Lespinet]

Prepare a configuration section for the git am documentation.
Remove the part related to the am.keepcr configuration variable in the
description of the --keepcr option and place the description of the
am.keepcr configuration variable in the newly created configuration
section.

This section will be used in the next commit.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
---
 Documentation/git-am.txt | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 0d8ba48..d412f6b 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -24,6 +24,16 @@ Splits mail messages in a mailbox into commit log message,
 authorship information and patches, and applies them to the
 current branch.
 
+CONFIGURATION
+-------------
+
+am.keepcr::
+	If true, git-am will call git-mailsplit for patches in mbox format
+	with parameter '--keep-cr'. In this case git-mailsplit will
+	not remove `\r` from lines ending with `\r\n`. Can be overridden
+	by giving '--no-keep-cr' from the command line.
+	See linkgit:git-am[1], linkgit:git-mailsplit[1].
+
 OPTIONS
 -------
 (<mbox>|<Maildir>)...::
@@ -43,11 +53,11 @@ OPTIONS
 --keep-non-patch::
 	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
 
---[no-]keep-cr::
+--keep-cr::
+--no-keep-cr::
 	With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
 	with the same option, to prevent it from stripping CR at the end of
-	lines. `am.keepcr` configuration variable can be used to specify the
-	default behaviour.  `--no-keep-cr` is useful to override `am.keepcr`.
+	lines.
 
 -c::
 --scissors::
-- 
1.9.1

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Junio C Hamano]

Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:

> Prepare a configuration section for the git am documentation.
> Remove the part related to the am.keepcr configuration variable in the
> description of the --keepcr option and place the description of the
> am.keepcr configuration variable in the newly created configuration
> section.
>
> This section will be used in the next commit.
>
> Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
> ---
>  Documentation/git-am.txt | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index 0d8ba48..d412f6b 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -24,6 +24,16 @@ Splits mail messages in a mailbox into commit log message,
>  authorship information and patches, and applies them to the
>  current branch.
>  
> +CONFIGURATION
> +-------------
> +
> +am.keepcr::
> +	If true, git-am will call git-mailsplit for patches in mbox format
> +	with parameter '--keep-cr'. In this case git-mailsplit will
> +	not remove `\r` from lines ending with `\r\n`. Can be overridden
> +	by giving '--no-keep-cr' from the command line.
> +	See linkgit:git-am[1], linkgit:git-mailsplit[1].

As this is git-am.txt, the first reference smells a bit odd.

Other than that, looks OK to me.

Thanks.

>  OPTIONS
>  -------
>  (<mbox>|<Maildir>)...::
> @@ -43,11 +53,11 @@ OPTIONS
>  --keep-non-patch::
>  	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
>  
> ---[no-]keep-cr::
> +--keep-cr::
> +--no-keep-cr::
>  	With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
>  	with the same option, to prevent it from stripping CR at the end of
> -	lines. `am.keepcr` configuration variable can be used to specify the
> -	default behaviour.  `--no-keep-cr` is useful to override `am.keepcr`.
> +	lines.
>  
>  -c::
>  --scissors::

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Torsten Bögershausen]

On 06/02/2015 07:34 PM, Remi Lespinet wrote:
> []
> Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
> ---
>   Documentation/git-am.txt | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index 0d8ba48..d412f6b 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -24,6 +24,16 @@ Splits mail messages in a mailbox into commit log message,
>   authorship information and patches, and applies them to the
>   current branch.
>   
> +CONFIGURATION
> +-------------
> +
> +am.keepcr::
> +	If true, git-am will call git-mailsplit for patches in mbox format
> +	with parameter '--keep-cr'. In this case git-mailsplit will
> +	not remove `\r` from lines ending with `\r\n`. Can be overridden
> +	by giving '--no-keep-cr' from the command line.
(This documentation assumes that am.keepcr is true)
Would it be clearer to put the "overridden" into one line and write like 
this:

Can be overridden by giving '--no-keep-cr' or '--keep-cr' from the command line.

[PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Remi Lespinet]

> On 06/03/2015 07:50 AM Torsten Bögershausen <tboegi@web.de> wrote
>
> > +CONFIGURATION
> > +-------------
> > +
> > +am.keepcr::
> > +        If true, git-am will call git-mailsplit for patches in mbox format
> > +        with parameter '--keep-cr'. In this case git-mailsplit will
> > +        not remove `\r` from lines ending with `\r\n`. Can be overridden
> > +        by giving '--no-keep-cr' from the command line.
> (This documentation assumes that am.keepcr is true)
> Would it be clearer to put the "overridden" into one line and write like
> this:
> 
> Can be overridden by giving '--no-keep-cr' or '--keep-cr' from the command line.

Yes I agree, or maybe:

'--keep-cr' and '--no-keep-cr' take precedence over this variable.

Actually, I don't know if we need to write it (as Paul Tan suggested
in the previous version of this patch for the threeway option
http://article.gmane.org/gmane.comp.version-control.git/270150)

I checked the documentation of different commands. From what I've
seen, such indications either does not appear or are right after the
text. I agree that it's a good idea, but for the sake of consistency,
I'd rather use one of these two format as long as it's ok for you.

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Torsten Bögershausen]

I checked the documentation of different commands. From what I've
seen, such indications either does not appear or are right after the
text. I agree that it's a good idea, but for the sake of consistency,
I'd rather use one of these two format as long as it's ok for you.


After re-checking: OK for me, sorry for the noise

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Matthieu Moy]

Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:

> Prepare a configuration section for the git am documentation.

Most git-*.txt do not have this CONFIGURATION section.

In an ideal world, we would have such section automatically generated
(i.e. the description for each variable would exist in one place, and we
would make sure that both "man git-config" and "man git-<command>" show
it). In a really ideal world, it would also be propagated to the code
and we would have a "git config --describe am.keepcr" or so that would
return the doc.

I'm a bit worried to see documentation cut-and-pasted from config.txt to
git-*.txt for maintainability: if the text on one side is modified,
we're likely to forget the other and the text will diverge with time.

Not a strong objection, but I have the feeling that the more we do this
kind of patches, the harder it will be if ever we decide to do the above.

> +CONFIGURATION
> +-------------
> +
> +am.keepcr::
> +	If true, git-am will call git-mailsplit for patches in mbox format

`git am`
`git mailsplit`

> +	with parameter '--keep-cr'. In this case git-mailsplit will

Likewise

> +	not remove `\r` from lines ending with `\r\n`. Can be overridden
> +	by giving '--no-keep-cr' from the command line.

That should be backquote, not forward-quote, right?

I know it's not your code since it's a cut-and-paste from config.txt,
but that illustrates my point above: we used to have one place with
wrong quotes, and we'd have two after the patch.

>  OPTIONS
>  -------
>  (<mbox>|<Maildir>)...::
> @@ -43,11 +53,11 @@ OPTIONS
>  --keep-non-patch::
>  	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
>  
> ---[no-]keep-cr::
> +--keep-cr::
> +--no-keep-cr::
>  	With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
>  	with the same option, to prevent it from stripping CR at the end of
> -	lines. `am.keepcr` configuration variable can be used to specify the
> -	default behaviour.

I don't think it's a good idea to remove this part. If I look for a way
to make --keep-cr the default, the first place I'd look would be the doc
for --keep-cr, and I'd appreciate a link to am.keepcr.

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

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>>  OPTIONS
>>  -------
>>  (<mbox>|<Maildir>)...::
>> @@ -43,11 +53,11 @@ OPTIONS
>>  --keep-non-patch::
>>  	Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
>>  
>> ---[no-]keep-cr::
>> +--keep-cr::
>> +--no-keep-cr::
>>  	With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
>>  	with the same option, to prevent it from stripping CR at the end of
>> -	lines. `am.keepcr` configuration variable can be used to specify the
>> -	default behaviour.
>
> I don't think it's a good idea to remove this part. If I look for a way
> to make --keep-cr the default, the first place I'd look would be the doc
> for --keep-cr, and I'd appreciate a link to am.keepcr.

Yup, very sensible suggestion.  Configuration gives default, command
line options override the default.  People come two different ends;
having to describe the same thing twice is wasteful, but will help
them one indirection to find relevant facts in the documentation.

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Remi Lespinet]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes

> Most git-*.txt do not have this CONFIGURATION section.
> 
> In an ideal world, we would have such section automatically generated
> (i.e. the description for each variable would exist in one place, and we
> would make sure that both "man git-config" and "man git-<command>" show
> it). In a really ideal world, it would also be propagated to the code
> and we would have a "git config --describe am.keepcr" or so that would
> return the doc.
> 
> I'm a bit worried to see documentation cut-and-pasted from config.txt to
> git-*.txt for maintainability: if the text on one side is modified,
> we're likely to forget the other and the text will diverge with time.
> 
> Not a strong objection, but I have the feeling that the more we do this
> kind of patches, the harder it will be if ever we decide to do the above.

I've seen occurences of this (mainly git-rebase.txt and
git-grep), but I agree, I think I'll remove the configuration
section.

> > +CONFIGURATION
> > +-------------
> > +
> > +am.keepcr::
> > +        If true, git-am will call git-mailsplit for patches in mbox format
> 
> `git am`
> `git mailsplit`
> 
> > +        with parameter '--keep-cr'. In this case git-mailsplit will
> 
> Likewise
> 
> > +        not remove `\r` from lines ending with `\r\n`. Can be overridden
> > +        by giving '--no-keep-cr' from the command line.
> 
> That should be backquote, not forward-quote, right?
> 
> I know it's not your code since it's a cut-and-paste from config.txt,
> but that illustrates my point above: we used to have one place with
> wrong quotes, and we'd have two after the patch.

Ok I'll correct it in a minor patch

Re: [PATCH/RFC v3 2/4] git-am.txt: add configuration section in git am documentation

[Remi Lespinet]

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes
> > > +CONFIGURATION
> > > +-------------
> > > +
> > > +am.keepcr::
> > > +        If true, git-am will call git-mailsplit for patches in mbox format
> >
> > `git am`
> > `git mailsplit`
> >
> > > +        with parameter '--keep-cr'. In this case git-mailsplit will
> >
> > Likewise
> >
> > > +        not remove `\r` from lines ending with `\r\n`. Can be overridden
> > > +        by giving '--no-keep-cr' from the command line.
> >
> > That should be backquote, not forward-quote, right?
> >
> > I know it's not your code since it's a cut-and-paste from config.txt,
> > but that illustrates my point above: we used to have one place with
> > wrong quotes, and we'd have two after the patch.
> 
> Ok I'll correct it in a minor patch

Actually I don't think that this is a good idea to correct
that (since there's many occurences of forward-quoted options in
git-config.txt). I'll just remove the configuration part in
the git am documentation.

[PATCH/RFC v3 3/4] t4150-am: refactor am -3 tests

[Remi Lespinet]

Create a setup for git am -3 in a separate test instead of creating
this setup each time.

This prepares for the next commit which will use this setup as well.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
---
 t/t4150-am.sh | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 306e6f3..6ced98c 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -274,15 +274,21 @@ test_expect_success 'am --keep-non-patch really keeps the non-patch part' '
 	grep "^\[foo\] third" actual
 '
 
+test_expect_success 'setup am -3' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout -b base3way master2 &&
+	sed -n -e "3,\$p" msg >file &&
+	head -n 9 msg >>file &&
+	git add file &&
+	test_tick &&
+	git commit -m "copied stuff"
+'
+
 test_expect_success 'am -3 falls back to 3-way merge' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-	git checkout -b lorem2 master2 &&
-	sed -n -e "3,\$p" msg >file &&
-	head -n 9 msg >>file &&
-	git add file &&
-	test_tick &&
-	git commit -m "copied stuff" &&
+	git checkout -b lorem2 base3way &&
 	git am -3 lorem-move.patch &&
 	test_path_is_missing .git/rebase-apply &&
 	git diff --exit-code lorem
@@ -291,12 +297,7 @@ test_expect_success 'am -3 falls back to 3-way merge' '
 test_expect_success 'am -3 -p0 can read --no-prefix patch' '
 	rm -fr .git/rebase-apply &&
 	git reset --hard &&
-	git checkout -b lorem3 master2 &&
-	sed -n -e "3,\$p" msg >file &&
-	head -n 9 msg >>file &&
-	git add file &&
-	test_tick &&
-	git commit -m "copied stuff" &&
+	git checkout -b lorem3 base3way &&
 	git am -3 -p0 lorem-zero.patch &&
 	test_path_is_missing .git/rebase-apply &&
 	git diff --exit-code lorem
@@ -338,12 +339,7 @@ test_expect_success 'am -3 can rename a file after falling back to 3-way merge'
 test_expect_success 'am -3 -q is quiet' '
 	rm -fr .git/rebase-apply &&
 	git checkout -f lorem2 &&
-	git reset master2 --hard &&
-	sed -n -e "3,\$p" msg >file &&
-	head -n 9 msg >>file &&
-	git add file &&
-	test_tick &&
-	git commit -m "copied stuff" &&
+	git reset base3way --hard &&
 	git am -3 -q lorem-move.patch >output.out 2>&1 &&
 	! test -s output.out
 '
-- 
1.9.1

[PATCH/RFC v3 4/4] git-am: add am.threeWay config variable

[Remi Lespinet]

Add the am.threeWay configuration variable to use the -3 or --3way
option of git am by default. When am.threeway is set and not desired
for a specific git am command, the --no-3way option can be used to
override it.

Signed-off-by: Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>
---
 Even if git am will be rewritten soon, the code that will have to be
 ported is not the most important part of the patch and the tests and
 documentation parts can be reused.

 Documentation/config.txt |  8 ++++++++
 Documentation/git-am.txt | 11 ++++++++++-
 git-am.sh                |  9 +++++++++
 t/t4150-am.sh            | 19 +++++++++++++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d44bc85..669f5fc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -769,6 +769,14 @@ am.keepcr::
 	by giving '--no-keep-cr' from the command line.
 	See linkgit:git-am[1], linkgit:git-mailsplit[1].
 
+am.threeWay::
+	By default, git-am will fail if the patch does not apply cleanly. When
+	set to true, this setting tells git-am to fall back on 3-way merge if
+	the patch records the identity of blobs it is supposed to apply to and
+	we have those blobs available locally (equivalent to giving the --3way
+	option from the command line).
+	See linkgit:git-am[1].
+
 apply.ignoreWhitespace::
 	When set to 'change', tells 'git apply' to ignore changes in
 	whitespace, in the same way as the '--ignore-space-change'
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index d412f6b..0472182 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
-	 [--3way] [--interactive] [--committer-date-is-author-date]
+	 [--[no-]3way] [--interactive] [--committer-date-is-author-date]
 	 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
 	 [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
@@ -34,6 +34,14 @@ am.keepcr::
 	by giving '--no-keep-cr' from the command line.
 	See linkgit:git-am[1], linkgit:git-mailsplit[1].
 
+am.threeWay::
+	By default, git-am will fail if the patch does not apply cleanly. When
+	set to true, this setting tells git-am to fall back on 3-way merge if
+	the patch records the identity of blobs it is supposed to apply to and
+	we have those blobs available locally (equivalent to giving the --3way
+	option from the command line).
+	See linkgit:git-am[1].
+
 OPTIONS
 -------
 (<mbox>|<Maildir>)...::
@@ -100,6 +108,7 @@ default.   You can use `--no-utf8` to override this.
 
 -3::
 --3way::
+--no-3way::
 	When the patch does not apply cleanly, fall back on
 	3-way merge if the patch records the identity of blobs
 	it is supposed to apply to and we have those blobs
diff --git a/git-am.sh b/git-am.sh
index c460dd0..75e701a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -390,6 +390,11 @@ then
     keepcr=t
 fi
 
+if test "$(git config --bool --get am.threeWay)" = true
+then
+    threeway=t
+fi
+
 while test $# != 0
 do
 	case "$1" in
@@ -401,6 +406,8 @@ it will be removed. Please do not use it anymore."
 		;;
 	-3|--3way)
 		threeway=t ;;
+	--no-3way)
+		threeway=f ;;
 	-s|--signoff)
 		sign=t ;;
 	-u|--utf8)
@@ -658,6 +665,8 @@ fi
 if test "$(cat "$dotest/threeway")" = t
 then
 	threeway=t
+else
+	threeway=f
 fi
 git_apply_opt=$(cat "$dotest/apply-opt")
 if test "$(cat "$dotest/sign")" = t
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 6ced98c..b822a39 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -303,6 +303,25 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' '
 	git diff --exit-code lorem
 '
 
+test_expect_success 'am with config am.threeWay falls back to 3-way merge' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout -b lorem4 base3way &&
+	test_config am.threeWay 1 &&
+	git am lorem-move.patch &&
+	test_path_is_missing .git/rebase-apply &&
+	git diff --exit-code lorem
+'
+
+test_expect_success 'am with config am.threeWay overridden by --no-3way' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard &&
+	git checkout -b lorem5 base3way &&
+	test_config am.threeWay 1 &&
+	test_must_fail git am --no-3way lorem-move.patch &&
+	test_path_is_dir .git/rebase-apply
+'
+
 test_expect_success 'am can rename a file' '
 	grep "^rename from" rename.patch &&
 	rm -fr .git/rebase-apply &&
-- 
1.9.1

Re: [PATCH/RFC v3 4/4] git-am: add am.threeWay config variable

[Junio C Hamano]

Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:

> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index d412f6b..0472182 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> ...
> +am.threeWay::
> +	By default, git-am will fail if the patch does not apply cleanly. When
> +	set to true, this setting tells git-am to fall back on 3-way merge if
> +	the patch records the identity of blobs it is supposed to apply to and
> +	we have those blobs available locally (equivalent to giving the --3way
> +	option from the command line).
> +	See linkgit:git-am[1].

Same comment as 2/4 applies here.

Re: [PATCH/RFC v3 4/4] git-am: add am.threeWay config variable

[Matthieu Moy]

Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:

> +am.threeWay::
> +	By default, git-am will fail if the patch does not apply cleanly.

http://article.gmane.org/gmane.comp.version-control.git/270538

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

[PATCH/RFC v3 4/4] git-am: add am.threeWay config variable

[Remi Lespinet]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes

> Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:
> 
> > +am.threeWay::
> > +        By default, git-am will fail if the patch does not apply cleanly.
> 
> http://article.gmane.org/gmane.comp.version-control.git/270538

Ok sorry.

Thanks

Re: [PATCH/RFC v3 1/4] git-am.sh: fix initialization of the threeway variable

[Junio C Hamano]

Overall this round looks sensible, modulo minor nits.

Thanks.