[PATCH 0/6] git-merge: a few documentation improvements

[PATCH 0/6] git-merge: a few documentation improvements

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

Sergey Organov (6):
  git-merge: clarify "usage" by adding "-m <msg>"
  Documentation/git-merge.txt: remove list of options from SYNOPSIS
  Documentation/git-merge.txt: fix SYNOPSIS of obsolete form to include
    options
  Documentation/git-merge.txt: improve short description in NAME
  Documentation/git-merge.txt: improve short description in DESCRIPTION
  Documentation/git-merge.txt: get rid of irrelevant references to
    git-pull

 Documentation/git-merge.txt | 63 ++++++++++++++++++++++-----------------------
 builtin/merge.c             |  2 +-
 2 files changed, 32 insertions(+), 33 deletions(-)

-- 
2.10.0.1.g57b01a3

[PATCH 1/6] git-merge: clarify "usage" by adding "-m <msg>"

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

"-m <msg>" is one of essential distinctions between obsolete
invocation form and the recent one. Add it to the "usage" returned by
'git merge -h' for more clarity.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 builtin/merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a8b57c7..0e367ba 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -43,7 +43,7 @@ struct strategy {
 };
 
 static const char * const builtin_merge_usage[] = {
-	N_("git merge [<options>] [<commit>...]"),
+	N_("git merge [<options>] [-m <msg>] [<commit>...]"),
 	N_("git merge [<options>] <msg> HEAD <commit>"),
 	N_("git merge --abort"),
 	NULL
-- 
2.10.0.1.g57b01a3

Re: [PATCH 1/6] git-merge: clarify "usage" by adding "-m <msg>"

[Junio C Hamano]

sorganov@gmail.com writes:

> From: Sergey Organov <sorganov@gmail.com>
>
> "-m <msg>" is one of essential distinctions between obsolete
> invocation form and the recent one. Add it to the "usage" returned by
> 'git merge -h' for more clarity.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  builtin/merge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a8b57c7..0e367ba 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -43,7 +43,7 @@ struct strategy {
>  };
>  
>  static const char * const builtin_merge_usage[] = {
> -	N_("git merge [<options>] [<commit>...]"),
> +	N_("git merge [<options>] [-m <msg>] [<commit>...]"),
>  	N_("git merge [<options>] <msg> HEAD <commit>"),
>  	N_("git merge --abort"),
>  	NULL

While this is not wrong per-se, as the deprecated form will go away
soon, I hope you do not mind if I had to drop this one from the
series to avoid merge conflicts to 'pu' (I do not know how bad the
conflict would be yet; I am just reviewing in my MUA).

Re: [PATCH 1/6] git-merge: clarify "usage" by adding "-m <msg>"

[Sergey Organov]

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

> sorganov@gmail.com writes:
>
>> From: Sergey Organov <sorganov@gmail.com>
>>
>> "-m <msg>" is one of essential distinctions between obsolete
>> invocation form and the recent one. Add it to the "usage" returned by
>> 'git merge -h' for more clarity.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  builtin/merge.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/merge.c b/builtin/merge.c
>> index a8b57c7..0e367ba 100644
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -43,7 +43,7 @@ struct strategy {
>>  };
>>  
>>  static const char * const builtin_merge_usage[] = {
>> -	N_("git merge [<options>] [<commit>...]"),
>> +	N_("git merge [<options>] [-m <msg>] [<commit>...]"),
>>  	N_("git merge [<options>] <msg> HEAD <commit>"),
>>  	N_("git merge --abort"),
>>  	NULL
>
> While this is not wrong per-se, as the deprecated form will go away
> soon, I hope you do not mind if I had to drop this one from the
> series to avoid merge conflicts to 'pu' (I do not know how bad the
> conflict would be yet; I am just reviewing in my MUA).

Yeah, sure. I was not aware obsolete form description is to go away
soon.

[PATCH 2/6] Documentation/git-merge.txt: remove list of options from SYNOPSIS

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

This partial list of option is confusing as it lacks a lot of
available options. It also clutters the SYNOPSIS making differences
between forms of invocation less clear.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-merge.txt | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..90342eb 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -9,10 +9,7 @@ git-merge - Join two or more development histories together
 SYNOPSIS
 --------
 [verse]
-'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
-	[--[no-]allow-unrelated-histories]
-	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
+'git merge' [options] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
 
-- 
2.10.0.1.g57b01a3

Re: [PATCH 2/6] Documentation/git-merge.txt: remove list of options from SYNOPSIS

[Junio C Hamano]

sorganov@gmail.com writes:

> From: Sergey Organov <sorganov@gmail.com>
>
> This partial list of option is confusing as it lacks a lot of
> available options. It also clutters the SYNOPSIS making differences
> between forms of invocation less clear.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-merge.txt | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index b758d55..90342eb 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -9,10 +9,7 @@ git-merge - Join two or more development histories together
>  SYNOPSIS
>  --------
>  [verse]
> -'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
> -	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
> -	[--[no-]allow-unrelated-histories]
> -	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
> +'git merge' [options] [-m <msg>] [<commit>...]
>  'git merge' <msg> HEAD <commit>...
>  'git merge' --abort

Same comment as 1/6; as we'd hopefully be removing the deprecated
form soonish, it would probably make sense to leave only two, i.e.

	git merge [options] [<commit>...]
	git merge --abort

in synposis.

Re: [PATCH 2/6] Documentation/git-merge.txt: remove list of options from SYNOPSIS

[Sergey Organov]

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

> sorganov@gmail.com writes:
>
>> From: Sergey Organov <sorganov@gmail.com>
>>
>> This partial list of option is confusing as it lacks a lot of
>> available options. It also clutters the SYNOPSIS making differences
>> between forms of invocation less clear.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  Documentation/git-merge.txt | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index b758d55..90342eb 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -9,10 +9,7 @@ git-merge - Join two or more development histories together
>>  SYNOPSIS
>>  --------
>>  [verse]
>> -'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
>> -	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
>> -	[--[no-]allow-unrelated-histories]
>> -	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
>> +'git merge' [options] [-m <msg>] [<commit>...]
>>  'git merge' <msg> HEAD <commit>...
>>  'git merge' --abort
>
> Same comment as 1/6; as we'd hopefully be removing the deprecated
> form soonish, it would probably make sense to leave only two, i.e.
>
> 	git merge [options] [<commit>...]
> 	git merge --abort
>
> in synposis.

Same "yes" as in 1/6, obviously.

-- Sergey

[PATCH 3/6] Documentation/git-merge.txt: fix SYNOPSIS of obsolete form to include options

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-merge.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 90342eb..216d2f4 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [options] [-m <msg>] [<commit>...]
-'git merge' <msg> HEAD <commit>...
+'git merge' [options] <msg> HEAD <commit>...
 'git merge' --abort
 
 DESCRIPTION
-- 
2.10.0.1.g57b01a3

[PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

Old description not only raised the question of why the tool is called
git-merge rather than git-join, but "join histories" also sounds like
very simple operation, something like what "git-merge -s ours" does.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-merge.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 216d2f4..cc0329d 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -3,7 +3,8 @@ git-merge(1)
 
 NAME
 ----
-git-merge - Join two or more development histories together
+
+git-merge - Merge one or more branches to the current branch
 
 
 SYNOPSIS
-- 
2.10.0.1.g57b01a3

Re: [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME

[Junio C Hamano]

sorganov@gmail.com writes:

> From: Sergey Organov <sorganov@gmail.com>
>
> Old description not only raised the question of why the tool is called
> git-merge rather than git-join, but "join histories" also sounds like
> very simple operation, something like what "git-merge -s ours" does.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-merge.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 216d2f4..cc0329d 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -3,7 +3,8 @@ git-merge(1)
>  
>  NAME
>  ----
> -git-merge - Join two or more development histories together
> +
> +git-merge - Merge one or more branches to the current branch

This patch, evaluated by itself, looks like a regression in that it
tries to explain "merge" by using verb "merge", making it fuzzier to
those who do not yet know what a "merge" is.  That was why it tried
to explain "merge" as an operation to join histories.

However, the next one, 5/6, resurrects the "join history" in the
description part to help them, so the damage is not so severe when
we take them together.

I haven't formed firm opinion on this patch yet.

Re: [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME

[Sergey Organov]

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

> sorganov@gmail.com writes:
>
>> From: Sergey Organov <sorganov@gmail.com>
>>
>> Old description not only raised the question of why the tool is called
>> git-merge rather than git-join, but "join histories" also sounds like
>> very simple operation, something like what "git-merge -s ours" does.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  Documentation/git-merge.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index 216d2f4..cc0329d 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -3,7 +3,8 @@ git-merge(1)
>>  
>>  NAME
>>  ----
>> -git-merge - Join two or more development histories together
>> +
>> +git-merge - Merge one or more branches to the current branch
>
> This patch, evaluated by itself, looks like a regression in that it
> tries to explain "merge" by using verb "merge", making it fuzzier to
> those who do not yet know what a "merge" is.  That was why it tried
> to explain "merge" as an operation to join histories.

My thought was that "merge", the operation, is so well-known term that
it could well go into the NAME section without explanation.

Besides:

$ man merge
NAME
       merge - three-way file merge
[...]

Uses the same pattern.

>
> However, the next one, 5/6, resurrects the "join history" in the
> description part to help them, so the damage is not so severe when
> we take them together.

Damage? In SCM world we can track the issue back to:

$ man -k rcsmerge
rcsmerge (1)         - merge RCS revisions

-- Sergey

Re: [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME

[Jeff King]

On Wed, Oct 05, 2016 at 05:46:22PM +0300, sorganov@gmail.com wrote:

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 216d2f4..cc0329d 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -3,7 +3,8 @@ git-merge(1)
>  
>  NAME
>  ----
> -git-merge - Join two or more development histories together
> +
> +git-merge - Merge one or more branches to the current branch

I wonder if we should be more clear that you don't have to merge a
branch; you can merge any commit. I do agree that the original was
unnecessarily general. And I think "the current branch" is accurate
(technically it can be to a detached HEAD, but that is pedantry that
doesn't need to make it into the synopsis).

So maybe "Merge one or more commits into the current branch".  I guess
that is a bit vague, too. It is really "commit tips" or "lines of
development" that we are merging. Bringing them in of course brings in
many commits, but the "or more" there is meant to hint at multi-parent
merges.

So perhaps "one or more branches", while not completely accurate, is the
best we can do. I dunno.

-Peff

Re: [PATCH 4/6] Documentation/git-merge.txt: improve short description in NAME

[Sergey Organov]

Jeff King <peff@peff.net> writes:

> On Wed, Oct 05, 2016 at 05:46:22PM +0300, sorganov@gmail.com wrote:
>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index 216d2f4..cc0329d 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -3,7 +3,8 @@ git-merge(1)
>>  
>>  NAME
>>  ----
>> -git-merge - Join two or more development histories together
>> +
>> +git-merge - Merge one or more branches to the current branch
>
> I wonder if we should be more clear that you don't have to merge a
> branch; you can merge any commit. I do agree that the original was
> unnecessarily general. And I think "the current branch" is accurate
> (technically it can be to a detached HEAD, but that is pedantry that
> doesn't need to make it into the synopsis).
>
> So maybe "Merge one or more commits into the current branch".  I guess
> that is a bit vague, too. It is really "commit tips" or "lines of
> development" that we are merging. Bringing them in of course brings in
> many commits, but the "or more" there is meant to hint at multi-parent
> merges.
>
> So perhaps "one or more branches", while not completely accurate, is the
> best we can do. I dunno.

You've basically repeated my entire line of thinking that lead to the
patch.

-- Sergey.

[PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

Old description had a few problems:

- sounded as if commits have changes

- stated that changes are taken since some "divergence point"
  that was not defined.

New description rather uses "common ancestor" and "merge base",
definitions of which are easily discoverable in the rest of GIT
documentation.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-merge.txt | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index cc0329d..351b8fc 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -16,11 +16,16 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-Incorporates changes from the named commits (since the time their
-histories diverged from the current branch) into the current
-branch.  This command is used by 'git pull' to incorporate changes
-from another repository and can be used by hand to merge changes
-from one branch into another.
+
+Incorporates changes that lead to the named commits into the current
+branch, and joins corresponding histories. The best common ancestor of
+named commits and the current branch, called "merge base", is
+calculated, and then net changes taken from the merge base to
+the named commits are applied.
+
+This command is used by 'git pull' to incorporate changes from another
+repository, and can be used by hand to merge changes from one branch
+into another.
 
 Assume the following history exists and the current branch is
 "`master`":
@@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
     D---E---F---G master
 ------------
 
-Then "`git merge topic`" will replay the changes made on the
-`topic` branch since it diverged from `master` (i.e., `E`) until
-its current commit (`C`) on top of `master`, and record the result
-in a new commit along with the names of the two parent commits and
-a log message from the user describing the changes.
+Then "`git merge topic`" will replay the changes made on the `topic`
+branch since it diverged from `master` (i.e., `E`) until its current
+commit (`C`) on top of `master`, and record the result in a new commit
+along with references to the two parent commits and a log message from
+the user describing the changes.
 
 ------------
 	  A---B---C topic
-- 
2.10.0.1.g57b01a3

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Jakub Narębski]

W dniu 05.10.2016 o 16:46, sorganov@gmail.com pisze:
> From: Sergey Organov <sorganov@gmail.com>
> 
> Old description had a few problems:
> 
> - sounded as if commits have changes
> 
> - stated that changes are taken since some "divergence point"
>   that was not defined.
> 
> New description rather uses "common ancestor" and "merge base",
> definitions of which are easily discoverable in the rest of GIT
> documentation.

This is a step in a good direction, but it has a few issues.

> 
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-merge.txt | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index cc0329d..351b8fc 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -16,11 +16,16 @@ SYNOPSIS
>  
>  DESCRIPTION
>  -----------
> -Incorporates changes from the named commits (since the time their
> -histories diverged from the current branch) into the current
> -branch.  This command is used by 'git pull' to incorporate changes
> -from another repository and can be used by hand to merge changes
> -from one branch into another.
> +
> +Incorporates changes that lead to the named commits into the current
> +branch, and joins corresponding histories. The best common ancestor of
> +named commits and the current branch, called "merge base", is
> +calculated, and then net changes taken from the merge base to
> +the named commits are applied.

The first sentence is all right; it reads better than the original
without the introduced part in parentheses.  The only minor issue
is with "joins corresponding histories" - it is a good description,
but may imply that the branch we are merging vanishes: it doesn't.
But all in all, it is a good change.

Second sentence has some problems.  First, while it is a good idea
to use well defined term "merge base", I think writing "since the
time their histories diverged" or "(which is the point where histories
diverged)" would be a good plain language description; it was removed
entirely in the proposal.

Second, while "common ancestor" and "least common ancestor" are well
defined in mathematics of graphs, "best common ancestor" isn't...
but this is what git-merge-base(1) documentation uses.

Also, the "best common ancestor" doesn't need to be only one.  There
might be many such ancestors... though Git would generate then a
virtual best common ancestor thanks to recursive merge strategy.
And usually there is only one "best common ancestor", that is a single
merge base.  So this may need clarification, but it is not much of
a problem.

Third, and most important, is that "net changes taken from the merge
base to the named commits are applied" is simply not true.  The
`git merge` command does not reapply changes - that is what rebase
and cherry-pick do.  The merge operation uses 3-way merge strategy
(diff3) between merge-base, current branch, and merged commit.
That is, it finds differences between differences, and "applies"
that.

See "A Formal Investigation of Diff3" paper by Sanjeev Khanna,
Keshav Kunal, and Benjamin C. Pierce:
  http://www.cis.upenn.edu/~bcpierce/papers/diff3-short.pdf

I'm not sure how to explain it succintly.  Perhaps

  net changes between merge base to the current (merged into)
  branch and named commits are integrated

There is description of trivial 3-way merge somewhere in Git docs,
though in very unobvious place; we can link it.

> +
> +This command is used by 'git pull' to incorporate changes from another
> +repository, and can be used by hand to merge changes from one branch
> +into another.

Rather "can be used by 'git pull'", or "is used by 'git pull' (unless
configured otherwise)"...

Separating this information makes a very good sense.  Thanks.

>  
>  Assume the following history exists and the current branch is
>  "`master`":
> @@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
>      D---E---F---G master
>  ------------
>  
> -Then "`git merge topic`" will replay the changes made on the
> -`topic` branch since it diverged from `master` (i.e., `E`) until
> -its current commit (`C`) on top of `master`, and record the result
> -in a new commit along with the names of the two parent commits and
> -a log message from the user describing the changes.
> +Then "`git merge topic`" will replay the changes made on the `topic`
> +branch since it diverged from `master` (i.e., `E`) until its current
> +commit (`C`) on top of `master`, and record the result in a new commit
> +along with references to the two parent commits and a log message from
> +the user describing the changes.

What the happened here!?!  Please do not rewrap documentation, especially
not without changes!

>  
>  ------------
>  	  A---B---C topic
> 
-- 
Jakub Narębski

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Junio C Hamano]

Jakub Narębski <jnareb@gmail.com> writes:

>> +
>> +This command is used by 'git pull' to incorporate changes from another
>> +repository, and can be used by hand to merge changes from one branch
>> +into another.
>
> Rather "can be used by 'git pull'", or "is used by 'git pull' (unless
> configured otherwise)"...

I think you are misreading the original and the update (see my
comments in the other message).

>> @@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
>>      D---E---F---G master
>>  ------------
>>  
>> -Then "`git merge topic`" will replay the changes made on the
>> -`topic` branch since it diverged from `master` (i.e., `E`) until
>> -its current commit (`C`) on top of `master`, and record the result
>> -in a new commit along with the names of the two parent commits and
>> -a log message from the user describing the changes.
>> +Then "`git merge topic`" will replay the changes made on the `topic`
>> +branch since it diverged from `master` (i.e., `E`) until its current
>> +commit (`C`) on top of `master`, and record the result in a new commit
>> +along with references to the two parent commits and a log message from
>> +the user describing the changes.
>
> What the happened here!?!  Please do not rewrap documentation, especially
> not without changes!

Yes, reflowing is bad but you can spot the change from "along with
the names of the parent commits" to "along with references to the
parent commits" if you stare at it long enough ;-)

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

Jakub Narębski <jnareb@gmail.com> writes:

> W dniu 05.10.2016 o 16:46, sorganov@gmail.com pisze:
>> From: Sergey Organov <sorganov@gmail.com>
>> 
>> Old description had a few problems:
>> 
>> - sounded as if commits have changes
>> 
>> - stated that changes are taken since some "divergence point"
>>   that was not defined.
>> 
>> New description rather uses "common ancestor" and "merge base",
>> definitions of which are easily discoverable in the rest of GIT
>> documentation.
>
> This is a step in a good direction, but it has a few issues.

Thanks a lot for reviewing! I'll need time to read your reply carefully.

-- Sergey

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

Jakub Narębski <jnareb@gmail.com> writes:

> W dniu 05.10.2016 o 16:46, sorganov@gmail.com pisze:
>> From: Sergey Organov <sorganov@gmail.com>
>> 
>> Old description had a few problems:
>> 
>> - sounded as if commits have changes
>> 
>> - stated that changes are taken since some "divergence point"
>>   that was not defined.
>> 
>> New description rather uses "common ancestor" and "merge base",
>> definitions of which are easily discoverable in the rest of GIT
>> documentation.
>
> This is a step in a good direction, but it has a few issues.
>
>> 
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  Documentation/git-merge.txt | 25 +++++++++++++++----------
>>  1 file changed, 15 insertions(+), 10 deletions(-)
>> 
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index cc0329d..351b8fc 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -16,11 +16,16 @@ SYNOPSIS
>>  
>>  DESCRIPTION
>>  -----------
>> -Incorporates changes from the named commits (since the time their
>> -histories diverged from the current branch) into the current
>> -branch.  This command is used by 'git pull' to incorporate changes
>> -from another repository and can be used by hand to merge changes
>> -from one branch into another.
>> +
>> +Incorporates changes that lead to the named commits into the current
>> +branch, and joins corresponding histories. The best common ancestor of
>> +named commits and the current branch, called "merge base", is
>> +calculated, and then net changes taken from the merge base to
>> +the named commits are applied.
>
> The first sentence is all right; it reads better than the original
> without the introduced part in parentheses.  The only minor issue
> is with "joins corresponding histories" - it is a good description,
> but may imply that the branch we are merging vanishes: it doesn't.
> But all in all, it is a good change.

I've got "joins corresponding histories" from original NAME section, so
it's likely to be good enough.

>
> Second sentence has some problems.  First, while it is a good idea
> to use well defined term "merge base", I think writing "since the
> time their histories diverged" or "(which is the point where histories
> diverged)" would be a good plain language description; it was removed
> entirely in the proposal.

I was not sure about it myself, but it sounded as if it could be the
case that I might need to resolve the same conflicts again and again, as
changes are taken from some "divergence point" that is apparently fixed.

OTOH, "merge base" not only is well-defined term, but it also doesn't
sound as some fixed point in history.

>
> Second, while "common ancestor" and "least common ancestor" are well
> defined in mathematics of graphs, "best common ancestor" isn't...
> but this is what git-merge-base(1) documentation uses.

That's were I took it from indeed, git-merge-base manual page. I wanted
things we mention to be discoverable.

> Also, the "best common ancestor" doesn't need to be only one.  There
> might be many such ancestors... though Git would generate then a
> virtual best common ancestor thanks to recursive merge strategy.
> And usually there is only one "best common ancestor", that is a single
> merge base.  So this may need clarification, but it is not much of
> a problem.
>
> Third, and most important, is that "net changes taken from the merge
> base to the named commits are applied" is simply not true.  The
> `git merge` command does not reapply changes - that is what rebase
> and cherry-pick do.

"are applied" meant to be generic enough in this context to cover, say,
"-s ours" as well. Perhaps "are integrated" you suggest below, or "are
incorporated" that is already used in the beginning of DESCRIPTION, is
better indeed.

As for merge vs rebase (or cherry-pick), the only essential difference
for me is that merge applies net (cumulative, integrated) changes, while
rebase applies each change sequentially, one commit to another. Please
correct me if I'm wrong.

BTW, one can imagine "rebase" merge strategy: make a new chain of
commits on top of destination, then squash all that to get a new single
merge commit.

> The merge operation uses 3-way merge strategy (diff3) between
> merge-base, current branch, and merged commit. That is, it finds
> differences between differences, and "applies" that.

The problem is that at this point we need to cover other merge
strategies as well, so we can't writ the description as if 3-way merge
is the only strategy.

> See
> "A Formal Investigation of Diff3" paper by Sanjeev Khanna, Keshav
> Kunal, and Benjamin C. Pierce:
> http://www.cis.upenn.edu/~bcpierce/papers/diff3-short.pdf
>
> I'm not sure how to explain it succintly.  Perhaps
>
>   net changes between merge base to the current (merged into)
>   branch and named commits are integrated
>
> There is description of trivial 3-way merge somewhere in Git docs,
> though in very unobvious place; we can link it.

As it's "TRUE MERGE" section that mentions 3-way merge for the first
time, it'd be the natural place for the link, I think.

>> +
>> +This command is used by 'git pull' to incorporate changes from another
>> +repository, and can be used by hand to merge changes from one branch
>> +into another.
>
> Rather "can be used by 'git pull'", or "is used by 'git pull' (unless
> configured otherwise)"...
>
> Separating this information makes a very good sense.  Thanks.

Yeah, but I've rather deleted it entirely in 6/6.

-- Sergey.

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Junio C Hamano]

sorganov@gmail.com writes:

> From: Sergey Organov <sorganov@gmail.com>
>
> Old description had a few problems:
>
> - sounded as if commits have changes
>
> - stated that changes are taken since some "divergence point"
>   that was not defined.
>
> New description rather uses "common ancestor" and "merge base",
> definitions of which are easily discoverable in the rest of GIT
> documentation.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-merge.txt | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index cc0329d..351b8fc 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -16,11 +16,16 @@ SYNOPSIS
>  
>  DESCRIPTION
>  -----------
> -Incorporates changes from the named commits (since the time their
> -histories diverged from the current branch) into the current
> -branch.  This command is used by 'git pull' to incorporate changes
> -from another repository and can be used by hand to merge changes
> -from one branch into another.
> +
> +Incorporates changes that lead to the named commits into the current
> +branch, and joins corresponding histories. The best common ancestor of
> +named commits and the current branch, called "merge base", is
> +calculated, and then net changes taken from the merge base to
> +the named commits are applied.
> +
> +This command is used by 'git pull' to incorporate changes from another
> +repository, and can be used by hand to merge changes from one branch
> +into another.

Content change together with re-flowing the text makes it more
costly than necessary to review a change like this.  Please avoid
doing so in your future patches.

I like what the updated description says very much.  I however
wonder if "and can be used by hand..." is still appropriate, or
needs a bit of modernizing.  It feels a bit awkward by making it
sound as if 'git merge' is primarily an implementation detail of
'git pull' but it can also be used as the first-class command, which
used to be the case in the old days back when "git pull . other" was
also perfectly good way to merge the 'other' branch from your own
repository, but I think your update is meant to clarify that we no
longer live in that old world ;-)

> @@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
>      D---E---F---G master
>  ------------
>  
> -Then "`git merge topic`" will replay the changes made on the
> -`topic` branch since it diverged from `master` (i.e., `E`) until
> -its current commit (`C`) on top of `master`, and record the result
> -in a new commit along with the names of the two parent commits and
> -a log message from the user describing the changes.

> -Then "`git merge topic`" will replay the changes made on the `topic`
> -branch since it diverged from `master` (i.e., `E`) until its current
> -commit (`C`) on top of `master`, and record the result in a new commit
> -along with the names of the two parent commits and a log message from
> -the user describing the changes.

> +Then "`git merge topic`" will replay the changes made on the `topic`
> +branch since it diverged from `master` (i.e., `E`) until its current
> +commit (`C`) on top of `master`, and record the result in a new commit
> +along with references to the two parent commits and a log message from
> +the user describing the changes.

Content change together with re-flowing the text makes it more
costly than necessary to review a change like this.  Please avoid
doing so in your future patches.

I had to re-flow the original you removed to match how you flowed in
the updated one and stare at it for a while to spot that the only
change was to rephrase "the names of the parents" to "references to
the parents".  I do not know if the updated phrasing is better.  The
"name" in the original was meant to be a short-hand for "object name",
and I would support a change to spell it out to clarify; "reference"
can be a vague word that can mean different things in Git, and when
the word is given without context, most Git people would think that
the word refers to "refs", but that is definitely not what the new
commit records, so...

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

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

> sorganov@gmail.com writes:
>
>> From: Sergey Organov <sorganov@gmail.com>
>>
>> Old description had a few problems:
>>
>> - sounded as if commits have changes
>>
>> - stated that changes are taken since some "divergence point"
>>   that was not defined.
>>
>> New description rather uses "common ancestor" and "merge base",
>> definitions of which are easily discoverable in the rest of GIT
>> documentation.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  Documentation/git-merge.txt | 25 +++++++++++++++----------
>>  1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
>> index cc0329d..351b8fc 100644
>> --- a/Documentation/git-merge.txt
>> +++ b/Documentation/git-merge.txt
>> @@ -16,11 +16,16 @@ SYNOPSIS
>>  
>>  DESCRIPTION
>>  -----------
>> -Incorporates changes from the named commits (since the time their
>> -histories diverged from the current branch) into the current
>> -branch.  This command is used by 'git pull' to incorporate changes
>> -from another repository and can be used by hand to merge changes
>> -from one branch into another.
>> +
>> +Incorporates changes that lead to the named commits into the current
>> +branch, and joins corresponding histories. The best common ancestor of
>> +named commits and the current branch, called "merge base", is
>> +calculated, and then net changes taken from the merge base to
>> +the named commits are applied.
>> +
>> +This command is used by 'git pull' to incorporate changes from another
>> +repository, and can be used by hand to merge changes from one branch
>> +into another.
>
> Content change together with re-flowing the text makes it more
> costly than necessary to review a change like this.  Please avoid
> doing so in your future patches.

OK, I see. So, what is the best way to handle this? Immediately follow
content change patch with another patch that only re-flows?

> I like what the updated description says very much.  I however
> wonder if "and can be used by hand..." is still appropriate, or
> needs a bit of modernizing.  It feels a bit awkward by making it
> sound as if 'git merge' is primarily an implementation detail of
> 'git pull' but it can also be used as the first-class command, which
> used to be the case in the old days back when "git pull . other" was
> also perfectly good way to merge the 'other' branch from your own
> repository, but I think your update is meant to clarify that we no
> longer live in that old world ;-)

Yes, exactly, but 6/6 removes most of the mentions of git-pull from the
manual anyway, so I felt it better belongs there.

>
>> @@ -31,11 +36,11 @@ Assume the following history exists and the current branch is
>>      D---E---F---G master
>>  ------------
>>  
>> -Then "`git merge topic`" will replay the changes made on the
>> -`topic` branch since it diverged from `master` (i.e., `E`) until
>> -its current commit (`C`) on top of `master`, and record the result
>> -in a new commit along with the names of the two parent commits and
>> -a log message from the user describing the changes.
>
>> -Then "`git merge topic`" will replay the changes made on the `topic`
>> -branch since it diverged from `master` (i.e., `E`) until its current
>> -commit (`C`) on top of `master`, and record the result in a new commit
>> -along with the names of the two parent commits and a log message from
>> -the user describing the changes.
>
>> +Then "`git merge topic`" will replay the changes made on the `topic`
>> +branch since it diverged from `master` (i.e., `E`) until its current
>> +commit (`C`) on top of `master`, and record the result in a new commit
>> +along with references to the two parent commits and a log message from
>> +the user describing the changes.
>
> Content change together with re-flowing the text makes it more
> costly than necessary to review a change like this.  Please avoid
> doing so in your future patches.

Yeah, got it.

> I had to re-flow the original you removed to match how you flowed in
> the updated one and stare at it for a while to spot that the only
> change was to rephrase "the names of the parents" to "references to
> the parents".  I do not know if the updated phrasing is better.  The
> "name" in the original was meant to be a short-hand for "object name",
> and I would support a change to spell it out to clarify; "reference"
> can be a vague word that can mean different things in Git, and when
> the word is given without context, most Git people would think that
> the word refers to "refs", but that is definitely not what the new
> commit records, so...

I won't insist on the change, but "name" sounded wrong to me, and
"reference" was most general term I was able to come up with in this
context.

First, "name" somehow suggested that it could be the case that

$ git branch
* master
$ git merge topic

will store strings "master" and "topic" in the resulting commit.

Next, should one really be aware of "10.2 Git Internals - Git Objects"
to grok this part of the manual if it actually meant to be "object
name"? 

Last, if "reference" is not good enough and we get to internals anyway,
why not say SHA1 then?

-- Sergey

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> OK, I see. So, what is the best way to handle this? Immediately follow
> content change patch with another patch that only re-flows?

Or no reflowing at all.

>> the parents".  I do not know if the updated phrasing is better.  The
>> "name" in the original was meant to be a short-hand for "object name",
>> and I would support a change to spell it out to clarify; "reference"
>> can be a vague word that can mean different things in Git, and when
>> the word is given without context, most Git people would think that
>> the word refers to "refs", but that is definitely not what the new
>> commit records, so...
>
> I won't insist on the change, but "name" sounded wrong to me, and
> "reference" was most general term I was able to come up with in this
> context.
> ...
> Last, if "reference" is not good enough and we get to internals anyway,
> why not say SHA1 then?

Because that is still colloquial?  I think s/name/object name/ is
a sensible change, but not s/name/reference/.

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> OK, I see. So, what is the best way to handle this? Immediately follow
>> content change patch with another patch that only re-flows?
>
> Or no reflowing at all.
>
>>> the parents".  I do not know if the updated phrasing is better.  The
>>> "name" in the original was meant to be a short-hand for "object name",
>>> and I would support a change to spell it out to clarify; "reference"
>>> can be a vague word that can mean different things in Git, and when
>>> the word is given without context, most Git people would think that
>>> the word refers to "refs", but that is definitely not what the new
>>> commit records, so...
>>
>> I won't insist on the change, but "name" sounded wrong to me, and
>> "reference" was most general term I was able to come up with in this
>> context.
>> ...
>> Last, if "reference" is not good enough and we get to internals anyway,
>> why not say SHA1 then?
>
> Because that is still colloquial? I think s/name/object name/ is a
> sensible change, but not s/name/reference/.

No, "reference" is more sensible here than any of "name", "object name",
or "SHA-1", the same way as here:

$ git help glossary
[...]
chain
        A list of objects, where each object in the list contains a
        reference to its successor (for example, the successor of a
        commit could be one of its parents).
[...]
$

The resulting merge commit is an origin for 2 chains, so it stores 2
references to its successors. No need to be aware of any [object] names
to understand all this.

-- Sergey

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>>> Last, if "reference" is not good enough and we get to internals anyway,
>>> why not say SHA1 then?
>>
>> Because that is still colloquial? I think s/name/object name/ is a
>> sensible change, but not s/name/reference/.
>
> No, "reference" is more sensible here than any of "name", "object name",
> or "SHA-1", the same way as here:
>
> $ git help glossary
> [...]
> chain
>         A list of objects, where each object in the list contains a
>         reference to its successor (for example, the successor of a
>         commit could be one of its parents).
> [...]

The entry for "chain" and the description under discussion have
stress on different aspect, though.  The description of "chain" is
more general: an object refers to another object by referring to it,
by unspecified means.  The reason why it is left unspecified is
because the way a tree object refers to blobs and trees is different
from the way a commit object refers to its parents (the former has
object names of blobs and trees in the tree entries; the latter uses
"parent" entries in the object header part to record object names of
parent commits).  It wants to stress more on the fact that there is
some mechanism to associate one object to others, than how that
association/linkage is expressed.

The way the resulting commit is described in the original text of
"git merge" description stresses more on "how" by being a lot more
specific to commit objects.  It does not just say "refers to parents
(by unspecified means)"; instead it tries to say what exactly are
recorded, i.e. the parents are referred to by recording the object
names of them in a new commit object.  It stresses more on "how"
(because it can afford to be more specific, unlike the description
of more general concept of a "chain").

It may be debatable if we want to give the description of what is
exactly recorded at that point of the document, but I personally
think that the users deserve a chance to learn how a merge is
recorded in "git merge" documentation.

Re: [PATCH 5/6] Documentation/git-merge.txt: improve short description in DESCRIPTION

[Sergey Organov]

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>>>> Last, if "reference" is not good enough and we get to internals anyway,
>>>> why not say SHA1 then?
>>>
>>> Because that is still colloquial? I think s/name/object name/ is a
>>> sensible change, but not s/name/reference/.
>>
>> No, "reference" is more sensible here than any of "name", "object name",
>> or "SHA-1", the same way as here:
>>
>> $ git help glossary
>> [...]
>> chain
>>         A list of objects, where each object in the list contains a
>>         reference to its successor (for example, the successor of a
>>         commit could be one of its parents).
>> [...]
>
> The entry for "chain" and the description under discussion have
> stress on different aspect, though.  The description of "chain" is
> more general: an object refers to another object by referring to it,
> by unspecified means.  The reason why it is left unspecified is
> because the way a tree object refers to blobs and trees is different
> from the way a commit object refers to its parents (the former has
> object names of blobs and trees in the tree entries; the latter uses
> "parent" entries in the object header part to record object names of
> parent commits).  It wants to stress more on the fact that there is
> some mechanism to associate one object to others, than how that
> association/linkage is expressed.
>
> The way the resulting commit is described in the original text of
> "git merge" description stresses more on "how" by being a lot more
> specific to commit objects.  It does not just say "refers to parents
> (by unspecified means)"; instead it tries to say what exactly are
> recorded, i.e. the parents are referred to by recording the object
> names of them in a new commit object.  It stresses more on "how"
> (because it can afford to be more specific, unlike the description
> of more general concept of a "chain").

That's were our disagreement actually is, and that's what I've tried to
fix with s/name/reference/, and that's why I'm against s/name/object
name/.

Rather than being more (and more) specific at every opportunity, one
needs a good reason to get more specific. In this particular case,
general DAG terminology seems to be enough to describe git-merge
semantics, thus using GIT specifics is unfounded.

> It may be debatable if we want to give the description of what is
> exactly recorded at that point of the document,

Exactly. My point in this particular discussion is that details of
recording of references to parents don't belong here, even though to
tell the truth I think they don't belong to git _user_ documentation at
all.

> but I personally
> think that the users deserve a chance to learn how a merge is
> recorded in "git merge" documentation.

I doubt a user will gain anything from this sacred knowledge suddenly
being thrown on him when what she is looking for is understanding of
basic merge semantics in GIT.

That said, if you still disagree, please feel free to just drop the
patch.

-- Sergey

[PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[sorganov]

From: Sergey Organov <sorganov@gmail.com>

No awareness of git-pull is required to understand git-merge operation,
so leave reference to git-pull only where it actually makes sense, in
the description of fast-forward merges, and only as clarification of
when this merging behaviour is mostly useful.

Other references to git-pull are likely just a historical leftover
that are now neither required nor clarify anything. Besides, git-pull
may use rebase rather than merge, so it's also technically wrong to
say, unconditionally, that git-pull uses git-merge.

Overall, let git-pull description refer to git-merge where
appropriate, and not vice versa.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-merge.txt | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 351b8fc..ba5fb0a 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -23,10 +23,6 @@ named commits and the current branch, called "merge base", is
 calculated, and then net changes taken from the merge base to
 the named commits are applied.
 
-This command is used by 'git pull' to incorporate changes from another
-repository, and can be used by hand to merge changes from one branch
-into another.
-
 Assume the following history exists and the current branch is
 "`master`":
 
@@ -119,18 +115,17 @@ of `git fetch` for merging are merged to the current branch.
 PRE-MERGE CHECKS
 ----------------
 
-Before applying outside changes, you should get your own work in
-good shape and committed locally, so it will not be clobbered if
-there are conflicts.  See also linkgit:git-stash[1].
-'git pull' and 'git merge' will stop without doing anything when
-local uncommitted changes overlap with files that 'git pull'/'git
-merge' may need to update.
+Before applying outside changes, you should get your own work in good
+shape and committed locally, so it will not be clobbered if there are
+conflicts. See also linkgit:git-stash[1]. 'git merge' will stop
+without doing anything when local uncommitted changes overlap with
+files that 'git merge' may need to update.
 
-To avoid recording unrelated changes in the merge commit,
-'git pull' and 'git merge' will also abort if there are any changes
-registered in the index relative to the `HEAD` commit.  (One
-exception is when the changed index entries are in the state that
-would result from the merge already.)
+To avoid recording unrelated changes in the merge commit, 'git merge'
+will also abort if there are any changes registered in the index
+relative to the `HEAD` commit. (One exception is when the changed
+index entries are in the state that would result from the merge
+already.)
 
 If all named commits are already ancestors of `HEAD`, 'git merge'
 will exit early with the message "Already up-to-date."
@@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
 FAST-FORWARD MERGE
 ------------------
 
-Often the current branch head is an ancestor of the named commit.
+Often the current branch head is an ancestor of the named commit.  In
+this case, a new commit is not needed to store the combined history;
+instead, the `HEAD` (along with the index) is updated to point at the
+named commit, without creating an extra merge commit.
+
 This is the most common case especially when invoked from 'git
 pull': you are tracking an upstream repository, you have committed
 no local changes, and now you want to update to a newer upstream
-revision.  In this case, a new commit is not needed to store the
-combined history; instead, the `HEAD` (along with the index) is
-updated to point at the named commit, without creating an extra
-merge commit.
+revision.
 
 This behavior can be suppressed with the `--no-ff` option.
 
-- 
2.10.0.1.g57b01a3

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

sorganov@gmail.com writes:

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 351b8fc..ba5fb0a 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -23,10 +23,6 @@ named commits and the current branch, called "merge base", is
>  calculated, and then net changes taken from the merge base to
>  the named commits are applied.
>  
> -This command is used by 'git pull' to incorporate changes from another
> -repository, and can be used by hand to merge changes from one branch
> -into another.
> -

Good.

> @@ -119,18 +115,17 @@ of `git fetch` for merging are merged to the current branch.
>  PRE-MERGE CHECKS
>  ----------------
>  
> -Before applying outside changes, you should get your own work in
> -good shape and committed locally, so it will not be clobbered if
> -there are conflicts.  See also linkgit:git-stash[1].
> -'git pull' and 'git merge' will stop without doing anything when
> -local uncommitted changes overlap with files that 'git pull'/'git
> -merge' may need to update.
> +Before applying outside changes, you should get your own work in good
> +shape and committed locally, so it will not be clobbered if there are
> +conflicts. See also linkgit:git-stash[1]. 'git merge' will stop
> +without doing anything when local uncommitted changes overlap with
> +files that 'git merge' may need to update.
>  
> -To avoid recording unrelated changes in the merge commit,
> -'git pull' and 'git merge' will also abort if there are any changes
> -registered in the index relative to the `HEAD` commit.  (One
> -exception is when the changed index entries are in the state that
> -would result from the merge already.)
> +To avoid recording unrelated changes in the merge commit, 'git merge'
> +will also abort if there are any changes registered in the index
> +relative to the `HEAD` commit. (One exception is when the changed
> +index entries are in the state that would result from the merge
> +already.)

OK, so "git pull and git merge" have been updated to say "git
merge" and there is no other change.  Looks good.

Please do not re-flow and change in the same commit, by the way.

> @@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
>  FAST-FORWARD MERGE
>  ------------------
>  
> -Often the current branch head is an ancestor of the named commit.
> +Often the current branch head is an ancestor of the named commit.  In
> +this case, a new commit is not needed to store the combined history;
> +instead, the `HEAD` (along with the index) is updated to point at the
> +named commit, without creating an extra merge commit.
> +
>  This is the most common case especially when invoked from 'git
>  pull': you are tracking an upstream repository, you have committed
>  no local changes, and now you want to update to a newer upstream
> -revision.  In this case, a new commit is not needed to store the
> -combined history; instead, the `HEAD` (along with the index) is
> -updated to point at the named commit, without creating an extra
> -merge commit.
> +revision.

I am not sure if the post-image of this hunk is better than the
original.

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

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

> sorganov@gmail.com writes:

[...]

>> @@ -138,14 +133,15 @@ will exit early with the message "Already up-to-date."
>>  FAST-FORWARD MERGE
>>  ------------------
>>  
>> -Often the current branch head is an ancestor of the named commit.
>> +Often the current branch head is an ancestor of the named commit.  In
>> +this case, a new commit is not needed to store the combined history;
>> +instead, the `HEAD` (along with the index) is updated to point at the
>> +named commit, without creating an extra merge commit.
>> +
>>  This is the most common case especially when invoked from 'git
>>  pull': you are tracking an upstream repository, you have committed
>>  no local changes, and now you want to update to a newer upstream
>> -revision.  In this case, a new commit is not needed to store the
>> -combined history; instead, the `HEAD` (along with the index) is
>> -updated to point at the named commit, without creating an extra
>> -merge commit.
>> +revision.
>
> I am not sure if the post-image of this hunk is better than the
> original.

That's what I've tried to explain in the description of the patch:

"No awareness of git-pull is required to understand git-merge operation,
so leave reference to git-pull only where it actually makes sense, in
the description of fast-forward merges, and only as clarification of
when this merging behaviour is mostly useful."

So I believe this change is inline with the rest of the patch. The
reference to git-pull (if it remains) should be a side-note, not part of
explanation of operation.

-- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> So I believe this change is inline with the rest of the patch. The
> reference to git-pull (if it remains) should be a side-note, not part of
> explanation of operation.

Not really.  The thing is, "This is the most common" needs to be
close to "Often...".  "git merge" directly invoked by the end user
is much less likely to encounter a fast forward situation; getting
invoked indirectly by "git pull" makes it common.

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> So I believe this change is inline with the rest of the patch. The
>> reference to git-pull (if it remains) should be a side-note, not part of
>> explanation of operation.
>
> Not really.  The thing is, "This is the most common" needs to be
> close to "Often...".  "git merge" directly invoked by the end user
> is much less likely to encounter a fast forward situation; getting
> invoked indirectly by "git pull" makes it common.

Ah, I now see. I tried to keep the text intact as much as possible, and
only split it into description and a note. Well, how about this then:

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b758d55..479400f 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
 FAST-FORWARD MERGE
 ------------------
 
-Often the current branch head is an ancestor of the named commit.
-This is the most common case especially when invoked from 'git
-pull': you are tracking an upstream repository, you have committed
-no local changes, and now you want to update to a newer upstream
-revision.  In this case, a new commit is not needed to store the
+When current branch head is an ancestor of the named commit,
+a new commit is not needed to store the
 combined history; instead, the `HEAD` (along with the index) is
 updated to point at the named commit, without creating an extra
 merge commit.
 
+This is very common case when 'git merge' is invoked from 'git
+pull': you are tracking an upstream repository, you have committed
+no local changes, and now you want to update to a newer upstream
+revision.  
+
 This behavior can be suppressed with the `--no-ff` option.
 
 TRUE MERGE

-- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> Ah, I now see. I tried to keep the text intact as much as possible, and
> only split it into description and a note. Well, how about this then:

Much better than your earlier patch, but I am not sure if the
updated one is that much better compared to the original.

The pre- and post- state of this "how about this" patch essentially
say the same thing, and I suspect that the primary reason why you
think the post- state is easier to read is because you wrote it,
while the reason why I do not see much difference is because I
didn't write the updated one ;-).

I do find "In this case, ... store the combined history" in the
original a bit awkward to read, but most of that awkardness is
inherited by the updated text.  It may benefit from hinting why a
new commit is not needed a bit stronger.  Here is my attempt:

    When the commit we are merging is a descendant of the current
    HEAD, the history leading to the named commit can be, and by
    default is, taken as the combined history of the two.  Our
    history is "fast forwarded" to their history by updating `HEAD`
    along with the index to point at the named commit.

    This often happens when you are following along somebody else's
    work via "git pull" without doing your own development.

I think the awkwardness I felt in the original and your version is
gone from the above attempt, but I doubt that it is better over
either of them in any other way.

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index b758d55..479400f 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -135,15 +135,17 @@ will exit early with the message "Already up-to-date."
>  FAST-FORWARD MERGE
>  ------------------
>  
> -Often the current branch head is an ancestor of the named commit.
> -This is the most common case especially when invoked from 'git
> -pull': you are tracking an upstream repository, you have committed
> -no local changes, and now you want to update to a newer upstream
> -revision.  In this case, a new commit is not needed to store the
> +When current branch head is an ancestor of the named commit,
> +a new commit is not needed to store the
>  combined history; instead, the `HEAD` (along with the index) is
>  updated to point at the named commit, without creating an extra
>  merge commit.
>  
> +This is very common case when 'git merge' is invoked from 'git
> +pull': you are tracking an upstream repository, you have committed
> +no local changes, and now you want to update to a newer upstream
> +revision.  
> +
>  This behavior can be suppressed with the `--no-ff` option.
>  
>  TRUE MERGE
>
> -- Sergey

Re: [PATCH 6/6] Documentation/git-merge.txt: get rid of irrelevant references to git-pull

[Sergey Organov]

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

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Ah, I now see. I tried to keep the text intact as much as possible, and
>> only split it into description and a note. Well, how about this then:
>
> Much better than your earlier patch, but I am not sure if the
> updated one is that much better compared to the original.

It's not intended to be much better. It is aimed at single simple
target: get rid of git-pull from descriptions of operations of
git-merge.

I'd just remove those git-pull reference, the only one that is left
after the patch, but it looks like git-merge needs an excuse to have
fast-forward on by default, and that excuse is the common git-pull case.

[I'd prefer 'git-merge --ff' were called from 'git-pull' and --no-ff be
the default for git-merge, but that's not the case, so I left the
reference to git-pull intact.]

>
> The pre- and post- state of this "how about this" patch essentially
> say the same thing, and I suspect that the primary reason why you
> think the post- state is easier to read is because you wrote it,
> while the reason why I do not see much difference is because I
> didn't write the updated one ;-).
>
> I do find "In this case, ... store the combined history" in the
> original a bit awkward to read, but most of that awkardness is
> inherited by the updated text.  It may benefit from hinting why a
> new commit is not needed a bit stronger.  Here is my attempt:
>
>     When the commit we are merging is a descendant of the current
>     HEAD, the history leading to the named commit can be, and by
>     default is, taken as the combined history of the two.  Our
>     history is "fast forwarded" to their history by updating `HEAD`
>     along with the index to point at the named commit.
>
>     This often happens when you are following along somebody else's
>     work via "git pull" without doing your own development.
>
> I think the awkwardness I felt in the original and your version is
> gone from the above attempt, but I doubt that it is better over
> either of them in any other way.

This is entirely different matter, and should be a subject of another
patch, if any. My patch meant to only address git-pull references, with
as few changes as possible.

-- Sergey