All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Documentation updates: merge-strategies
@ 2021-08-03 15:35 Elijah Newren via GitGitGadget
  2021-08-03 15:35 ` [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
                   ` (11 more replies)
  0 siblings, 12 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren

I noticed while updating my switch-default-merge-strategy-to-ort submission,
that many of the changes were good documentation updates that we might want
for Git v2.33.0. So I pulled those changes out and split them into lots of
little commits so that if any parts need discussion or are objectionable, we
can just drop those from this series and apply the rest for v2.33.0.

The first 9 commits are just small documentation updates, but there is one
commit at the end that updates an error message and a code comment.

Elijah Newren (10):
  git-rebase.txt: correct antiquated claims about --rebase-merges
  directory-rename-detection.txt: small updates due to merge-ort
    optimizations
  Documentation: edit awkward references to `git merge-recursive`
  merge-strategies.txt: update wording for the resolve strategy
  merge-strategies.txt: do not imply using copy detection is desired
  merge-strategies.txt: avoid giving special preference to patience
    algorithm
  merge-strategies.txt: explain why no-renames might be useful
  merge-strategies.txt: fix simple capitalization error
  Documentation: add coverage of the `ort` merge strategy
  Update error message and code comment

 Documentation/git-rebase.txt                  | 29 ++++++----
 Documentation/merge-options.txt               |  4 +-
 Documentation/merge-strategies.txt            | 56 +++++++++++--------
 .../technical/directory-rename-detection.txt  | 14 +++--
 builtin/merge.c                               |  2 +-
 sequencer.c                                   |  2 +-
 6 files changed, 63 insertions(+), 44 deletions(-)


base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1059%2Fnewren%2Fort-doc-updates-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1059/newren/ort-doc-updates-v1
Pull-Request: https://github.com/git/git/pull/1059
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 22:53   ` Johannes Schindelin
  2021-08-03 15:35 ` [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

When --rebase-merges was first introduced, it only worked with the
`recursive` strategy.  Some time later, it gained support for merges
using the `octopus` strategy.  The limitation of only supporting these
two strategies was documented in 25cff9f109 ("rebase -i --rebase-merges:
add a section to the man page", 2018-04-25) and lifted in e145d99347
("rebase -r: support merge strategies other than `recursive`",
2019-07-31).  However, when the limitation was lifted, the documentation
was not updated.  Update it now.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 55af6fd24e2..8a67227846a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -1219,12 +1219,16 @@ successful merge so that the user can edit the message.
 If a `merge` command fails for any reason other than merge conflicts (i.e.
 when the merge operation did not even start), it is rescheduled immediately.
 
-At this time, the `merge` command will *always* use the `recursive`
-merge strategy for regular merges, and `octopus` for octopus merges,
-with no way to choose a different one. To work around
-this, an `exec` command can be used to call `git merge` explicitly,
-using the fact that the labels are worktree-local refs (the ref
-`refs/rewritten/onto` would correspond to the label `onto`, for example).
+By default, the `merge` command will use the `recursive` merge
+strategy for regular merges, and `octopus` for octopus merges.  One
+can specify a default strategy for all merges using the `--strategy`
+argument when invoking rebase, or can override specific merges in the
+interactive list of commands by using an `exec` command to call `git
+merge` explicitly with a `--strategy` argument.  Note that when
+calling `git merge` explicitly like this, you can make use of the fact
+that the labels are worktree-local refs (the ref `refs/rewritten/onto`
+would correspond to the label `onto`, for example) in order to refer
+to the branches you want to merge.
 
 Note: the first command (`label onto`) labels the revision onto which
 the commits are rebased; The name `onto` is just a convention, as a nod
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
  2021-08-03 15:35 ` [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-04  0:06   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

In commit 0c4fd732f0 ("Move computation of dir_rename_count from
merge-ort to diffcore-rename", 2021-02-27), much of the logic for
computing directory renames moved into diffcore-rename.
directory-rename-detection.txt had claims that all of that logic was
found in merge-recursive.  Update the documentation.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 .../technical/directory-rename-detection.txt       | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt
index 49b83ef3cc4..029ee2cedc5 100644
--- a/Documentation/technical/directory-rename-detection.txt
+++ b/Documentation/technical/directory-rename-detection.txt
@@ -2,9 +2,9 @@ Directory rename detection
 ==========================
 
 Rename detection logic in diffcore-rename that checks for renames of
-individual files is aggregated and analyzed in merge-recursive for cases
-where combinations of renames indicate that a full directory has been
-renamed.
+individual files is also aggregated there and then analyzed in either
+merge-ort or merge-recursive for cases where combinations of renames
+indicate that a full directory has been renamed.
 
 Scope of abilities
 ------------------
@@ -88,9 +88,11 @@ directory rename detection support in:
     Folks have requested in the past that `git diff` detect directory
     renames and somehow simplify its output.  It is not clear whether this
     would be desirable or how the output should be simplified, so this was
-    simply not implemented.  Further, to implement this, directory rename
-    detection logic would need to move from merge-recursive to
-    diffcore-rename.
+    simply not implemented.  Also, while diffcore-rename has most of the
+    logic for detecting directory renames, some of the logic is still found
+    within merge-ort and merge-recursive.  Fully supporting directory
+    rename detection in diffs would require copying or moving the remaining
+    bits of logic to the diff machinery.
 
   * am
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive`
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
  2021-08-03 15:35 ` [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
  2021-08-03 15:35 ` [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-04  0:14   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

A few places in the documentation referred to the "`recursive` strategy"
using the phrase "`git merge-recursive`", suggesting that it was forking
subprocesses to call a toplevel builtin.  Perhaps that was relevant to
when rebase was a shell script, but it seems like a rather indirect way
to refer to the `recursive` strategy.  Simplify the references.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt       | 4 ++--
 Documentation/merge-options.txt    | 4 ++--
 Documentation/merge-strategies.txt | 9 +++++----
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a67227846a..7044afba362 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -355,8 +355,8 @@ See also INCOMPATIBLE OPTIONS below.
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy.
-	If there is no `-s` option 'git merge-recursive' is used
-	instead.  This implies --merge.
+	If there is no `-s` option the `recursive` strategy is the
+	default. This implies --merge.
 +
 Because 'git rebase' replays each commit from the working branch
 on top of the <upstream> branch using the given strategy, using
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index eb0aabd396f..f819bd8dd68 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail.
 	Use the given merge strategy; can be supplied more than
 	once to specify them in the order they should be tried.
 	If there is no `-s` option, a built-in list of strategies
-	is used instead ('git merge-recursive' when merging a single
-	head, 'git merge-octopus' otherwise).
+	is used instead (`recursive` when merging a single head,
+	`octopus` otherwise).
 
 -X <option>::
 --strategy-option=<option>::
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 2912de706bf..5d707e952aa 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -51,10 +51,11 @@ patience;;
 	See also linkgit:git-diff[1] `--patience`.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
-	Tells 'merge-recursive' to use a different diff algorithm, which
-	can help avoid mismerges that occur due to unimportant matching
-	lines (such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Defaults to the
+	`diff.algorithm` config setting.
 
 ignore-space-change;;
 ignore-all-space;;
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (2 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-04  0:19   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

The resolve merge strategy was given prominent positioning in this
document, being listed first since it was the default at the time the
document was written.  It hasn't been the default since before Git v1.0
was released, though.  Move it later in the document, near `octopus` and
`ours`.

Further, the wording for "resolve" claimed that it was "considered
generally safe and fast", which implies that the other strategies are
not.  While such an implication may have been true in 2005 when written,
it may well be that `ort` is faster today (since it does not need to
recurse into all directories).  Also, since `resolve` was the default
for less than a year while `recursive` has been the default for a decade
and a half, I think `recursive` is more battle-tested than `resolve` is.
Just strike this extraneous phrase.

Also, provide some quick historical context that may help users
understand its purpose and place in the list of merge strategies.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 5d707e952aa..6b6017e1cc8 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
 can also take their own options, which can be passed by giving `-X<option>`
 arguments to `git merge` and/or `git pull`.
 
-resolve::
-	This can only resolve two heads (i.e. the current branch
-	and another branch you pulled from) using a 3-way merge
-	algorithm.  It tries to carefully detect criss-cross
-	merge ambiguities and is considered generally safe and
-	fast.
-
 recursive::
 	This can only resolve two heads using a 3-way merge
 	algorithm.  When there is more than one common
@@ -106,6 +99,13 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+resolve::
+	This can only resolve two heads (i.e. the current branch
+	and another branch you pulled from) using a 3-way merge
+	algorithm.  It tries to carefully detect criss-cross
+	merge ambiguities.  It cannot handle renames.  This was
+	the default merge algorithm prior to November 2005.
+
 octopus::
 	This resolves cases with more than two heads, but refuses to do
 	a complex merge that needs manual resolution.  It is
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (3 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 22:56   ` Johannes Schindelin
  2021-08-04  0:21   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Stating that the recursive strategy "currently cannot make use of
detected copies" implies that this is a technical shortcoming of the
current algorithm.  I disagree with that.  I don't see how copies could
possibly be used in a sane fashion in a merge algorithm -- would we
propagate changes in one file on one side of history to each copy of
that file when merging?  That makes no sense to me.  I cannot think of
anything else that would make sense either.  Change the wording to
simply state that we ignore any copies.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 6b6017e1cc8..bc82799365a 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -16,9 +16,9 @@ recursive::
 	causing mismerges by tests done on actual merge commits
 	taken from Linux 2.6 kernel development history.
 	Additionally this can detect and handle merges involving
-	renames, but currently cannot make use of detected
-	copies.  This is the default merge strategy when pulling
-	or merging one branch.
+	renames.  It does not make use of detected copies.  This
+	is the default merge strategy when pulling or merging one
+	branch.
 +
 The 'recursive' strategy can take the following options:
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (4 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 17:00   ` Eric Sunshine
  2021-08-03 15:35 ` [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful Elijah Newren via GitGitGadget
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We already have diff-algorithm that explains why there are special diff
algorithms, so we do not need to re-explain patience.  patience exists
as its own toplevel option for historical reasons, but there's no reason
to give it special preference or document it again and suggest it's more
important than other diff algorithms, so just refer to it as a
deprecated shorthand for `diff-algorithm=patience`.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index bc82799365a..eb43befac7b 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -37,11 +37,7 @@ theirs;;
 	no 'theirs' merge strategy to confuse this merge option with.
 
 patience;;
-	With this option, 'merge-recursive' spends a little extra time
-	to avoid mismerges that sometimes occur due to unimportant
-	matching lines (e.g., braces from distinct functions).  Use
-	this when the branches to be merged have diverged wildly.
-	See also linkgit:git-diff[1] `--patience`.
+	Deprecated shorthand for diff-algorithm=patience.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
 	Use a different diff algorithm while merging, which can help
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (5 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-04  0:28   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 08/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index eb43befac7b..d21dbd1e051 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -75,9 +75,10 @@ no-renormalize;;
 	`merge.renormalize` configuration variable.
 
 no-renames;;
-	Turn off rename detection. This overrides the `merge.renames`
-	configuration variable.
-	See also linkgit:git-diff[1] `--no-renames`.
+	Turn off rename detection, which can be computationally
+	expensive.  This overrides the `merge.renames`
+	configuration variable.  See also linkgit:git-diff[1]
+	`--no-renames`.
 
 find-renames[=<n>];;
 	Turn on rename detection, optionally setting the similarity
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 08/10] merge-strategies.txt: fix simple capitalization error
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (6 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 23:01   ` Johannes Schindelin
  2021-08-03 15:35 ` [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7044afba362..b4429954480 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -530,7 +530,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
 where commits can be reordered, inserted and dropped at will.
 +
 It is currently only possible to recreate the merge commits using the
-`recursive` merge strategy; Different merge strategies can be used only via
+`recursive` merge strategy; different merge strategies can be used only via
 explicit `exec git merge -s <strategy> [...]` commands.
 +
 See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (7 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 08/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 23:03   ` Johannes Schindelin
  2021-08-04  0:33   ` Junio C Hamano
  2021-08-03 15:35 ` [PATCH 10/10] Update error message and code comment Elijah Newren via GitGitGadget
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt       |  7 ++++---
 Documentation/merge-strategies.txt | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index b4429954480..3e112011c6f 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -340,9 +340,10 @@ See also INCOMPATIBLE OPTIONS below.
 
 -m::
 --merge::
-	Use merging strategies to rebase.  When the recursive (default) merge
-	strategy is used, this allows rebase to be aware of renames on the
-	upstream side.  This is the default.
+	Use merging strategies to rebase.  When either the `recursive`
+	(default) or `ort` merge strategy is used, this allows rebase
+	to be aware of renames on the upstream side.  This is the
+	default.
 +
 Note that a rebase merge works by replaying each commit from the working
 branch on top of the <upstream> branch.  Because of this, when a merge
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index d21dbd1e051..d13d4a29875 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -96,6 +96,20 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+ort::
+	This is meant as a drop-in replacement for the `recursive`
+	algorithm (as reflected in its acronym -- "Ostensibly
+	Recursive's Twin"), and will likely replace it in the future.
+	It fixes corner cases that the `recursive` strategy handles
+	suboptimally, and is significantly faster in large
+	repositories -- especially when many renames are involved.
++
+The `ort` strategy takes all the same options as `recursive`.
+However, it ignores three of those options: `no-renames`,
+`patience` and `diff-algorithm`.  It always runs with rename
+detection (it handles it much faster than `recursive` does), and
+it specifically uses diff-algorithm=histogram.
+
 resolve::
 	This can only resolve two heads (i.e. the current branch
 	and another branch you pulled from) using a 3-way merge
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH 10/10] Update error message and code comment
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (8 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
@ 2021-08-03 15:35 ` Elijah Newren via GitGitGadget
  2021-08-03 23:05   ` Johannes Schindelin
  2021-08-03 23:06 ` [PATCH 00/10] Documentation updates: merge-strategies Johannes Schindelin
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
  11 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-03 15:35 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

There were two locations in the code that referred to 'merge-recursive'
but which were also applicable to 'merge-ort'.  Update them to more
general wording.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/merge.c | 2 +-
 sequencer.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a8a843b1f54..24b62a9c532 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		for (x = 0; x < xopts_nr; x++)
 			if (parse_merge_opt(&o, xopts[x]))
-				die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
+				die(_("Unknown strategy option: -X%s"), xopts[x]);
 
 		o.branch1 = head_arg;
 		o.branch2 = merge_remote_util(remoteheads->item)->name;
diff --git a/sequencer.c b/sequencer.c
index 7f07cd00f3f..a4e5c43fcf2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2065,7 +2065,7 @@ static int do_pick_commit(struct repository *r,
 		/*
 		 * We do not intend to commit immediately.  We just want to
 		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
+		 * that represents the "current" state for the merge machinery
 		 * to work on.
 		 */
 		if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 61+ messages in thread

* Re: [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm
  2021-08-03 15:35 ` [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
@ 2021-08-03 17:00   ` Eric Sunshine
  0 siblings, 0 replies; 61+ messages in thread
From: Eric Sunshine @ 2021-08-03 17:00 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: Git List, Elijah Newren

On Tue, Aug 3, 2021 at 11:35 AM Elijah Newren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> We already have diff-algorithm that explains why there are special diff
> algorithms, so we do not need to re-explain patience.  patience exists
> as its own toplevel option for historical reasons, but there's no reason
> to give it special preference or document it again and suggest it's more
> important than other diff algorithms, so just refer to it as a
> deprecated shorthand for `diff-algorithm=patience`.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> @@ -37,11 +37,7 @@ theirs;;
>  patience;;
> -       With this option, 'merge-recursive' spends a little extra time
> -       to avoid mismerges that sometimes occur due to unimportant
> -       matching lines (e.g., braces from distinct functions).  Use
> -       this when the branches to be merged have diverged wildly.
> -       See also linkgit:git-diff[1] `--patience`.
> +       Deprecated shorthand for diff-algorithm=patience.

Probably want to wrap backticks around `diff-algorithm=patience`. The
rest of this file seems to be pretty consistent about it. Indeed, the
existing deprecation in this file does so:

    rename-threshold=<n>;;
        Deprecated synonym for `find-renames=<n>`.

Maybe also s/shorthand/synonym/ for consistency with the existing
deprecation notice.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges
  2021-08-03 15:35 ` [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
@ 2021-08-03 22:53   ` Johannes Schindelin
  0 siblings, 0 replies; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 22:53 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> When --rebase-merges was first introduced, it only worked with the
> `recursive` strategy.  Some time later, it gained support for merges
> using the `octopus` strategy.  The limitation of only supporting these
> two strategies was documented in 25cff9f109 ("rebase -i --rebase-merges:
> add a section to the man page", 2018-04-25) and lifted in e145d99347
> ("rebase -r: support merge strategies other than `recursive`",
> 2019-07-31).  However, when the limitation was lifted, the documentation
> was not updated.  Update it now.

ACK!

Thank you,
Dscho

>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-rebase.txt | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 55af6fd24e2..8a67227846a 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -1219,12 +1219,16 @@ successful merge so that the user can edit the message.
>  If a `merge` command fails for any reason other than merge conflicts (i.e.
>  when the merge operation did not even start), it is rescheduled immediately.
>
> -At this time, the `merge` command will *always* use the `recursive`
> -merge strategy for regular merges, and `octopus` for octopus merges,
> -with no way to choose a different one. To work around
> -this, an `exec` command can be used to call `git merge` explicitly,
> -using the fact that the labels are worktree-local refs (the ref
> -`refs/rewritten/onto` would correspond to the label `onto`, for example).
> +By default, the `merge` command will use the `recursive` merge
> +strategy for regular merges, and `octopus` for octopus merges.  One
> +can specify a default strategy for all merges using the `--strategy`
> +argument when invoking rebase, or can override specific merges in the
> +interactive list of commands by using an `exec` command to call `git
> +merge` explicitly with a `--strategy` argument.  Note that when
> +calling `git merge` explicitly like this, you can make use of the fact
> +that the labels are worktree-local refs (the ref `refs/rewritten/onto`
> +would correspond to the label `onto`, for example) in order to refer
> +to the branches you want to merge.
>
>  Note: the first command (`label onto`) labels the revision onto which
>  the commits are rebased; The name `onto` is just a convention, as a nod
> --
> gitgitgadget
>
>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired
  2021-08-03 15:35 ` [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
@ 2021-08-03 22:56   ` Johannes Schindelin
  2021-08-04  0:21   ` Junio C Hamano
  1 sibling, 0 replies; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 22:56 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Stating that the recursive strategy "currently cannot make use of
> detected copies" implies that this is a technical shortcoming of the
> current algorithm.  I disagree with that.  I don't see how copies could
> possibly be used in a sane fashion in a merge algorithm -- would we
> propagate changes in one file on one side of history to each copy of
> that file when merging?  That makes no sense to me.  I cannot think of
> anything else that would make sense either.  Change the wording to
> simply state that we ignore any copies.

FWIW I fully agree with this reasoning.

Ciao,
Dscho

>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/merge-strategies.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> index 6b6017e1cc8..bc82799365a 100644
> --- a/Documentation/merge-strategies.txt
> +++ b/Documentation/merge-strategies.txt
> @@ -16,9 +16,9 @@ recursive::
>  	causing mismerges by tests done on actual merge commits
>  	taken from Linux 2.6 kernel development history.
>  	Additionally this can detect and handle merges involving
> -	renames, but currently cannot make use of detected
> -	copies.  This is the default merge strategy when pulling
> -	or merging one branch.
> +	renames.  It does not make use of detected copies.  This
> +	is the default merge strategy when pulling or merging one
> +	branch.
>  +
>  The 'recursive' strategy can take the following options:
>
> --
> gitgitgadget
>
>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/10] merge-strategies.txt: fix simple capitalization error
  2021-08-03 15:35 ` [PATCH 08/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
@ 2021-08-03 23:01   ` Johannes Schindelin
  2021-08-03 23:32     ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 23:01 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-rebase.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 7044afba362..b4429954480 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -530,7 +530,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
>  where commits can be reordered, inserted and dropped at will.
>  +
>  It is currently only possible to recreate the merge commits using the
> -`recursive` merge strategy; Different merge strategies can be used only via
> +`recursive` merge strategy; different merge strategies can be used only via

I am not a native speaker, so I'm eager to learn what is the correct thing
to do here. In particular since I continued in lower-case after a
semicolon for _years_, right up until some native speaker mentioned that
that's only correct if I continue with an incomplete sentence. If a
complete sentence follows the semicolon, so the advice went, I should
start the sentence with an upper-case letter.

Could you help me understand the correct rules here?

Thanks,
Dscho

>  explicit `exec git merge -s <strategy> [...]` commands.
>  +
>  See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
> --
> gitgitgadget
>
>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy
  2021-08-03 15:35 ` [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
@ 2021-08-03 23:03   ` Johannes Schindelin
  2021-08-03 23:43     ` Elijah Newren
  2021-08-04  0:33   ` Junio C Hamano
  1 sibling, 1 reply; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 23:03 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-rebase.txt       |  7 ++++---
>  Documentation/merge-strategies.txt | 14 ++++++++++++++
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index b4429954480..3e112011c6f 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -340,9 +340,10 @@ See also INCOMPATIBLE OPTIONS below.
>
>  -m::
>  --merge::
> -	Use merging strategies to rebase.  When the recursive (default) merge
> -	strategy is used, this allows rebase to be aware of renames on the
> -	upstream side.  This is the default.
> +	Use merging strategies to rebase.  When either the `recursive`
> +	(default) or `ort` merge strategy is used, this allows rebase
> +	to be aware of renames on the upstream side.  This is the
> +	default.

Since this now talks about two merge strategies, I think "This is the
default" needs to specify which of the two strategies is the default.

>  +
>  Note that a rebase merge works by replaying each commit from the working
>  branch on top of the <upstream> branch.  Because of this, when a merge
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> index d21dbd1e051..d13d4a29875 100644
> --- a/Documentation/merge-strategies.txt
> +++ b/Documentation/merge-strategies.txt
> @@ -96,6 +96,20 @@ subtree[=<path>];;
>  	is prefixed (or stripped from the beginning) to make the shape of
>  	two trees to match.
>
> +ort::
> +	This is meant as a drop-in replacement for the `recursive`
> +	algorithm (as reflected in its acronym -- "Ostensibly
> +	Recursive's Twin"), and will likely replace it in the future.
> +	It fixes corner cases that the `recursive` strategy handles
> +	suboptimally, and is significantly faster in large
> +	repositories -- especially when many renames are involved.
> ++
> +The `ort` strategy takes all the same options as `recursive`.
> +However, it ignores three of those options: `no-renames`,
> +`patience` and `diff-algorithm`.  It always runs with rename
> +detection (it handles it much faster than `recursive` does), and
> +it specifically uses diff-algorithm=histogram.

Probably `diff-algorithm=histogram` should also be enclosed within
backticks.

Thanks,
Dscho

> +
>  resolve::
>  	This can only resolve two heads (i.e. the current branch
>  	and another branch you pulled from) using a 3-way merge
> --
> gitgitgadget
>
>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 10/10] Update error message and code comment
  2021-08-03 15:35 ` [PATCH 10/10] Update error message and code comment Elijah Newren via GitGitGadget
@ 2021-08-03 23:05   ` Johannes Schindelin
  2021-08-03 23:49     ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 23:05 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> There were two locations in the code that referred to 'merge-recursive'
> but which were also applicable to 'merge-ort'.  Update them to more
> general wording.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  builtin/merge.c | 2 +-
>  sequencer.c     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a8a843b1f54..24b62a9c532 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
>
>  		for (x = 0; x < xopts_nr; x++)
>  			if (parse_merge_opt(&o, xopts[x]))
> -				die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
> +				die(_("Unknown strategy option: -X%s"), xopts[x]);

Since we updated our rules to start `die()` messages with a lower-case
letter, we could sneak in this change here, too. That would save
translators one extra round.

Thank you,
Dscho

>
>  		o.branch1 = head_arg;
>  		o.branch2 = merge_remote_util(remoteheads->item)->name;
> diff --git a/sequencer.c b/sequencer.c
> index 7f07cd00f3f..a4e5c43fcf2 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2065,7 +2065,7 @@ static int do_pick_commit(struct repository *r,
>  		/*
>  		 * We do not intend to commit immediately.  We just want to
>  		 * merge the differences in, so let's compute the tree
> -		 * that represents the "current" state for merge-recursive
> +		 * that represents the "current" state for the merge machinery
>  		 * to work on.
>  		 */
>  		if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))
> --
> gitgitgadget
>

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 00/10] Documentation updates: merge-strategies
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (9 preceding siblings ...)
  2021-08-03 15:35 ` [PATCH 10/10] Update error message and code comment Elijah Newren via GitGitGadget
@ 2021-08-03 23:06 ` Johannes Schindelin
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
  11 siblings, 0 replies; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-03 23:06 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:

> I noticed while updating my switch-default-merge-strategy-to-ort submission,
> that many of the changes were good documentation updates that we might want
> for Git v2.33.0. So I pulled those changes out and split them into lots of
> little commits so that if any parts need discussion or are objectionable, we
> can just drop those from this series and apply the rest for v2.33.0.
>
> The first 9 commits are just small documentation updates, but there is one
> commit at the end that updates an error message and a code comment.

I looked through them, and they all looked sensible. Hopefully my few
suggestions/questions are helpful.

Thank you for working on this,
Dscho

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/10] merge-strategies.txt: fix simple capitalization error
  2021-08-03 23:01   ` Johannes Schindelin
@ 2021-08-03 23:32     ` Elijah Newren
  2021-08-04 21:42       ` Johannes Schindelin
  0 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren @ 2021-08-03 23:32 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 5:01 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
> On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/git-rebase.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index 7044afba362..b4429954480 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -530,7 +530,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
> >  where commits can be reordered, inserted and dropped at will.
> >  +
> >  It is currently only possible to recreate the merge commits using the
> > -`recursive` merge strategy; Different merge strategies can be used only via
> > +`recursive` merge strategy; different merge strategies can be used only via
>
> I am not a native speaker, so I'm eager to learn what is the correct thing
> to do here. In particular since I continued in lower-case after a
> semicolon for _years_, right up until some native speaker mentioned that
> that's only correct if I continue with an incomplete sentence. If a
> complete sentence follows the semicolon, so the advice went, I should
> start the sentence with an upper-case letter.
>
> Could you help me understand the correct rules here?

First I've ever heard of it; I just presumed it was a typo.  I don't
pretend to be an expert on such topics (and although native speakers
may have an edge in knowing what "sounds natural", I tend to presume
we are less likely to know the rules).  However, some quick internet
searches suggest that different style guides differ on whether to
follow a *colon* with a capital letter[1][2][3], but everything I can
find suggests that *semicolons* should always be followed by a
lowercase letter (with exceptions for proper nouns)[2][4][5][and
others...].

[1] http://www.sussex.ac.uk/informatics/punctuation/colonandsemi/colon
[2] https://www.quora.com/Why-is-capitalization-necessary-after-a-semi-colon
[3] https://writingcenter.unc.edu/tips-and-tools/semi-colons-colons-and-dashes/
[4] https://writeanswers.royalroads.ca/faq/199268
[5] https://wordcounter.net/blog/2016/06/24/101812_capitalize-words-colon-semi-colon.html

So, after reading through the above links, I'm fairly convinced that
'different' should be lowercase here.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy
  2021-08-03 23:03   ` Johannes Schindelin
@ 2021-08-03 23:43     ` Elijah Newren
  0 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren @ 2021-08-03 23:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 5:04 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
> On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/git-rebase.txt       |  7 ++++---
> >  Documentation/merge-strategies.txt | 14 ++++++++++++++
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index b4429954480..3e112011c6f 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -340,9 +340,10 @@ See also INCOMPATIBLE OPTIONS below.
> >
> >  -m::
> >  --merge::
> > -     Use merging strategies to rebase.  When the recursive (default) merge
> > -     strategy is used, this allows rebase to be aware of renames on the
> > -     upstream side.  This is the default.
> > +     Use merging strategies to rebase.  When either the `recursive`
> > +     (default) or `ort` merge strategy is used, this allows rebase
> > +     to be aware of renames on the upstream side.  This is the
> > +     default.
>
> Since this now talks about two merge strategies, I think "This is the
> default" needs to specify which of the two strategies is the default.

It does, but I agree the wording is a bit confusing.  "This is the
default" refers to the fact that "Using merging strategies to rebase"
is the default (as opposed to using git-am).  We can then list some of
the possible merge strategies and which are the default (which we did
in the second sentence).  However, diving into a discussion about
individual strategies might be out of place here.  It looks like that
was originally mentioned because that allowed rebase to handle
renames, but git-am -3 gained that ability well over a decade ago via
falling back to the merge machinery.  So perhaps we should just
simplify this to:

"""
Using merging strategies to rebase (this is the default since Git
v2.26.0).  Note that there are multiple merge strategies available,
and specific ones can be picked with the `--strategy` option.
"""

>
> >  +
> >  Note that a rebase merge works by replaying each commit from the working
> >  branch on top of the <upstream> branch.  Because of this, when a merge
> > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> > index d21dbd1e051..d13d4a29875 100644
> > --- a/Documentation/merge-strategies.txt
> > +++ b/Documentation/merge-strategies.txt
> > @@ -96,6 +96,20 @@ subtree[=<path>];;
> >       is prefixed (or stripped from the beginning) to make the shape of
> >       two trees to match.
> >
> > +ort::
> > +     This is meant as a drop-in replacement for the `recursive`
> > +     algorithm (as reflected in its acronym -- "Ostensibly
> > +     Recursive's Twin"), and will likely replace it in the future.
> > +     It fixes corner cases that the `recursive` strategy handles
> > +     suboptimally, and is significantly faster in large
> > +     repositories -- especially when many renames are involved.
> > ++
> > +The `ort` strategy takes all the same options as `recursive`.
> > +However, it ignores three of those options: `no-renames`,
> > +`patience` and `diff-algorithm`.  It always runs with rename
> > +detection (it handles it much faster than `recursive` does), and
> > +it specifically uses diff-algorithm=histogram.
>
> Probably `diff-algorithm=histogram` should also be enclosed within
> backticks.

I'll fix that up.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 10/10] Update error message and code comment
  2021-08-03 23:05   ` Johannes Schindelin
@ 2021-08-03 23:49     ` Elijah Newren
  0 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren @ 2021-08-03 23:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 5:05 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi Elijah,
>
> On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > There were two locations in the code that referred to 'merge-recursive'
> > but which were also applicable to 'merge-ort'.  Update them to more
> > general wording.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  builtin/merge.c | 2 +-
> >  sequencer.c     | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/builtin/merge.c b/builtin/merge.c
> > index a8a843b1f54..24b62a9c532 100644
> > --- a/builtin/merge.c
> > +++ b/builtin/merge.c
> > @@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
> >
> >               for (x = 0; x < xopts_nr; x++)
> >                       if (parse_merge_opt(&o, xopts[x]))
> > -                             die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
> > +                             die(_("Unknown strategy option: -X%s"), xopts[x]);
>
> Since we updated our rules to start `die()` messages with a lower-case
> letter, we could sneak in this change here, too. That would save
> translators one extra round.

Sure, will do.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations
  2021-08-03 15:35 ` [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
@ 2021-08-04  0:06   ` Junio C Hamano
  0 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:06 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt
> index 49b83ef3cc4..029ee2cedc5 100644
> --- a/Documentation/technical/directory-rename-detection.txt
> +++ b/Documentation/technical/directory-rename-detection.txt
> @@ -2,9 +2,9 @@ Directory rename detection
>  ==========================
>  
>  Rename detection logic in diffcore-rename that checks for renames of
> -individual files is aggregated and analyzed in merge-recursive for cases
> -where combinations of renames indicate that a full directory has been
> -renamed.
> +individual files is also aggregated there and then analyzed in either
> +merge-ort or merge-recursive for cases where combinations of renames
> +indicate that a full directory has been renamed.
>  
>  Scope of abilities
>  ------------------
> @@ -88,9 +88,11 @@ directory rename detection support in:
>      Folks have requested in the past that `git diff` detect directory
>      renames and somehow simplify its output.  It is not clear whether this
>      would be desirable or how the output should be simplified, so this was
> -    simply not implemented.  Further, to implement this, directory rename
> -    detection logic would need to move from merge-recursive to
> -    diffcore-rename.
> +    simply not implemented.  Also, while diffcore-rename has most of the
> +    logic for detecting directory renames, some of the logic is still found
> +    within merge-ort and merge-recursive.  Fully supporting directory
> +    rename detection in diffs would require copying or moving the remaining
> +    bits of logic to the diff machinery.
>  
>    * am

Looks good.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive`
  2021-08-03 15:35 ` [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
@ 2021-08-04  0:14   ` Junio C Hamano
  2021-08-04  0:19     ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:14 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -355,8 +355,8 @@ See also INCOMPATIBLE OPTIONS below.
>  -s <strategy>::
>  --strategy=<strategy>::
>  	Use the given merge strategy.
> -	If there is no `-s` option 'git merge-recursive' is used
> -	instead.  This implies --merge.
> +	If there is no `-s` option the `recursive` strategy is the
> +	default. This implies --merge.

We can depart from the original even more to make it shorter and
more readable, I think.

    Use the given merge strategy, instead of the default
    `recursive`.  This implies `--merge`.

But the above is readable enough already, so I'll queue it as-is.


^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-03 15:35 ` [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
@ 2021-08-04  0:19   ` Junio C Hamano
  2021-08-04  0:37     ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:19 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> The resolve merge strategy was given prominent positioning in this
> document, being listed first since it was the default at the time the
> document was written.  It hasn't been the default since before Git v1.0
> was released, though.  Move it later in the document, near `octopus` and
> `ours`.

I think it was listed first because it was written first.

> Further, the wording for "resolve" claimed that it was "considered
> generally safe and fast", which implies that the other strategies are
> not.

I do not think it never implied any such thing; it is good to remove
it, or add the same to all strategies, though.

> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> index 5d707e952aa..6b6017e1cc8 100644
> --- a/Documentation/merge-strategies.txt
> +++ b/Documentation/merge-strategies.txt
> @@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
>  can also take their own options, which can be passed by giving `-X<option>`
>  arguments to `git merge` and/or `git pull`.
>  
> -resolve::
> -	This can only resolve two heads (i.e. the current branch
> -	and another branch you pulled from) using a 3-way merge
> -	algorithm.  It tries to carefully detect criss-cross
> -	merge ambiguities and is considered generally safe and
> -	fast.
> -
>  recursive::
>  	This can only resolve two heads using a 3-way merge
>  	algorithm.  When there is more than one common
> @@ -106,6 +99,13 @@ subtree[=<path>];;
>  	is prefixed (or stripped from the beginning) to make the shape of
>  	two trees to match.
>  
> +resolve::
> +	This can only resolve two heads (i.e. the current branch
> +	and another branch you pulled from) using a 3-way merge
> +	algorithm.  It tries to carefully detect criss-cross
> +	merge ambiguities.  It cannot handle renames.  This was
> +	the default merge algorithm prior to November 2005.

"It does not handle renames"; it wasn't like we wanted to do so and
cannot---we didn't want to, didn't think it was worth doing, it was
not part of the design objective to do renames, period.

I do not think it is even worth mentioning that it was the default
in the past.  And I do not think it is worth saying what timeframe
the recursive was the default, either.

>  octopus::
>  	This resolves cases with more than two heads, but refuses to do
>  	a complex merge that needs manual resolution.  It is

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive`
  2021-08-04  0:14   ` Junio C Hamano
@ 2021-08-04  0:19     ` Elijah Newren
  0 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren @ 2021-08-04  0:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 6:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > @@ -355,8 +355,8 @@ See also INCOMPATIBLE OPTIONS below.
> >  -s <strategy>::
> >  --strategy=<strategy>::
> >       Use the given merge strategy.
> > -     If there is no `-s` option 'git merge-recursive' is used
> > -     instead.  This implies --merge.
> > +     If there is no `-s` option the `recursive` strategy is the
> > +     default. This implies --merge.
>
> We can depart from the original even more to make it shorter and
> more readable, I think.
>
>     Use the given merge strategy, instead of the default
>     `recursive`.  This implies `--merge`.
>
> But the above is readable enough already, so I'll queue it as-is.

I'm making multiple tweaks already for Eric and Dscho, so I'll include
this in the re-roll.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired
  2021-08-03 15:35 ` [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
  2021-08-03 22:56   ` Johannes Schindelin
@ 2021-08-04  0:21   ` Junio C Hamano
  1 sibling, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:21 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -	renames, but currently cannot make use of detected
> -	copies.  This is the default merge strategy when pulling
> -	or merging one branch.
> +	renames.  It does not make use of detected copies.  This
> +	is the default merge strategy when pulling or merging one
> +	branch.

Good use of "does not"; we do not and we should not say "cannot"
here.


^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful
  2021-08-03 15:35 ` [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful Elijah Newren via GitGitGadget
@ 2021-08-04  0:28   ` Junio C Hamano
  2021-08-04  0:44     ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:28 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/merge-strategies.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> index eb43befac7b..d21dbd1e051 100644
> --- a/Documentation/merge-strategies.txt
> +++ b/Documentation/merge-strategies.txt
> @@ -75,9 +75,10 @@ no-renormalize;;
>  	`merge.renormalize` configuration variable.
>  
>  no-renames;;
> -	Turn off rename detection. This overrides the `merge.renames`
> -	configuration variable.
> -	See also linkgit:git-diff[1] `--no-renames`.
> +	Turn off rename detection, which can be computationally
> +	expensive.  This overrides the `merge.renames`
> +	configuration variable.  See also linkgit:git-diff[1]
> +	`--no-renames`.

Other reasons are that we may find a pair that the user did not
intend to when they made the change (i.e. it was done purely a
creation and a deletion but we found similarity), or we may find a
wrong original to consolidate changes from a side branch into, and
these are fundamental as it is our early design choice not to
record renames at the time of committing.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy
  2021-08-03 15:35 ` [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
  2021-08-03 23:03   ` Johannes Schindelin
@ 2021-08-04  0:33   ` Junio C Hamano
  2021-08-04  0:39     ` Elijah Newren
  1 sibling, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  0:33 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  --merge::
> -	Use merging strategies to rebase.  When the recursive (default) merge
> -	strategy is used, this allows rebase to be aware of renames on the
> -	upstream side.  This is the default.
> +	Use merging strategies to rebase.  When either the `recursive`
> +	(default) or `ort` merge strategy is used, this allows rebase
> +	to be aware of renames on the upstream side.  This is the
> +	default.

The "this is the default" at the end is about --merge vs format-patch|am,
so it should come near "Use merging strategies to rebase".

    Use merging strategies to rebase (default).  Renames on the
    upstream side is taken into account when the `recursive`
    (default) or `ort` merge strategy is used.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-04  0:19   ` Junio C Hamano
@ 2021-08-04  0:37     ` Elijah Newren
  2021-08-04  2:01       ` Junio C Hamano
  0 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren @ 2021-08-04  0:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 6:19 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > The resolve merge strategy was given prominent positioning in this
> > document, being listed first since it was the default at the time the
> > document was written.  It hasn't been the default since before Git v1.0
> > was released, though.  Move it later in the document, near `octopus` and
> > `ours`.
>
> I think it was listed first because it was written first.

Do you want it to be kept first for that reason as well?  I thought it
made more sense to cover the default strategy first, but I can move it
back if you prefer historical implementation order.

> > Further, the wording for "resolve" claimed that it was "considered
> > generally safe and fast", which implies that the other strategies are
> > not.
>
> I do not think it never implied any such thing; it is good to remove
> it, or add the same to all strategies, though.

I am unsure if your double negatives are intentional
(not..never)...but I think you're saying it's okay to remove this
text.

> > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> > index 5d707e952aa..6b6017e1cc8 100644
> > --- a/Documentation/merge-strategies.txt
> > +++ b/Documentation/merge-strategies.txt
> > @@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
> >  can also take their own options, which can be passed by giving `-X<option>`
> >  arguments to `git merge` and/or `git pull`.
> >
> > -resolve::
> > -     This can only resolve two heads (i.e. the current branch
> > -     and another branch you pulled from) using a 3-way merge
> > -     algorithm.  It tries to carefully detect criss-cross
> > -     merge ambiguities and is considered generally safe and
> > -     fast.
> > -
> >  recursive::
> >       This can only resolve two heads using a 3-way merge
> >       algorithm.  When there is more than one common
> > @@ -106,6 +99,13 @@ subtree[=<path>];;
> >       is prefixed (or stripped from the beginning) to make the shape of
> >       two trees to match.
> >
> > +resolve::
> > +     This can only resolve two heads (i.e. the current branch
> > +     and another branch you pulled from) using a 3-way merge
> > +     algorithm.  It tries to carefully detect criss-cross
> > +     merge ambiguities.  It cannot handle renames.  This was
> > +     the default merge algorithm prior to November 2005.
>
> "It does not handle renames"; it wasn't like we wanted to do so and
> cannot---we didn't want to, didn't think it was worth doing, it was
> not part of the design objective to do renames, period.

Right, sorry for the poor wording.  I'll fix it to use your phrase.

> I do not think it is even worth mentioning that it was the default
> in the past.  And I do not think it is worth saying what timeframe
> the recursive was the default, either.

I don't feel strongly about it and I can strike it.  My reasoning was
that providing historical timeframes might help them repeat or
recreate old merges in their own repositories.

However, I agree that the more likely bits of information that will
help users select the strategy they want are the capabilities (e.g.
renames, criss-cross merges, number of heads, etc.).

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy
  2021-08-04  0:33   ` Junio C Hamano
@ 2021-08-04  0:39     ` Elijah Newren
  0 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren @ 2021-08-04  0:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 6:33 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >  --merge::
> > -     Use merging strategies to rebase.  When the recursive (default) merge
> > -     strategy is used, this allows rebase to be aware of renames on the
> > -     upstream side.  This is the default.
> > +     Use merging strategies to rebase.  When either the `recursive`
> > +     (default) or `ort` merge strategy is used, this allows rebase
> > +     to be aware of renames on the upstream side.  This is the
> > +     default.
>
> The "this is the default" at the end is about --merge vs format-patch|am,
> so it should come near "Use merging strategies to rebase".
>
>     Use merging strategies to rebase (default).  Renames on the
>     upstream side is taken into account when the `recursive`
>     (default) or `ort` merge strategy is used.

We can simplify this to just:

    Use merging strategies to rebase (default).

because when git-am -3 was switched from `resolve` to `recursive` in
Dec 2006, it too gained the ability to handle renames and made the
rest of this description for --merge obsolete.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful
  2021-08-04  0:28   ` Junio C Hamano
@ 2021-08-04  0:44     ` Elijah Newren
  2021-08-04  2:05       ` Junio C Hamano
  0 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren @ 2021-08-04  0:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

On Tue, Aug 3, 2021 at 6:28 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/merge-strategies.txt | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> > index eb43befac7b..d21dbd1e051 100644
> > --- a/Documentation/merge-strategies.txt
> > +++ b/Documentation/merge-strategies.txt
> > @@ -75,9 +75,10 @@ no-renormalize;;
> >       `merge.renormalize` configuration variable.
> >
> >  no-renames;;
> > -     Turn off rename detection. This overrides the `merge.renames`
> > -     configuration variable.
> > -     See also linkgit:git-diff[1] `--no-renames`.
> > +     Turn off rename detection, which can be computationally
> > +     expensive.  This overrides the `merge.renames`
> > +     configuration variable.  See also linkgit:git-diff[1]
> > +     `--no-renames`.
>
> Other reasons are that we may find a pair that the user did not
> intend to when they made the change (i.e. it was done purely a
> creation and a deletion but we found similarity), or we may find a
> wrong original to consolidate changes from a side branch into, and
> these are fundamental as it is our early design choice not to
> record renames at the time of committing.

Good points.  Trying to describe all of that makes it somewhat
lengthy; does it make sense to include all of that, or should I just
drop this patch?

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-04  0:37     ` Elijah Newren
@ 2021-08-04  2:01       ` Junio C Hamano
  0 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  2:01 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

Elijah Newren <newren@gmail.com> writes:

> On Tue, Aug 3, 2021 at 6:19 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > From: Elijah Newren <newren@gmail.com>
>> >
>> > The resolve merge strategy was given prominent positioning in this
>> > document, being listed first since it was the default at the time the
>> > document was written.  It hasn't been the default since before Git v1.0
>> > was released, though.  Move it later in the document, near `octopus` and
>> > `ours`.
>>
>> I think it was listed first because it was written first.
>
> Do you want it to be kept first for that reason as well?  I thought it
> made more sense to cover the default strategy first, but I can move it
> back if you prefer historical implementation order.

No, I was only correcting the guess described in the log message.

> I am unsure if your double negatives are intentional
> (not..never)...but I think you're saying it's okay to remove this
> text.

Yes.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful
  2021-08-04  0:44     ` Elijah Newren
@ 2021-08-04  2:05       ` Junio C Hamano
  0 siblings, 0 replies; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  2:05 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

Elijah Newren <newren@gmail.com> writes:

>> >  no-renames;;
>> > -     Turn off rename detection. This overrides the `merge.renames`
>> > -     configuration variable.
>> > -     See also linkgit:git-diff[1] `--no-renames`.
>> > +     Turn off rename detection, which can be computationally
>> > +     expensive.  This overrides the `merge.renames`
>> > +     configuration variable.  See also linkgit:git-diff[1]
>> > +     `--no-renames`.
>>
>> Other reasons are that we may find a pair that the user did not
>> intend to when they made the change (i.e. it was done purely a
>> creation and a deletion but we found similarity), or we may find a
>> wrong original to consolidate changes from a side branch into, and
>> these are fundamental as it is our early design choice not to
>> record renames at the time of committing.
>
> Good points.  Trying to describe all of that makes it somewhat
> lengthy; does it make sense to include all of that, or should I just
> drop this patch?

What do the git-diff page say?  If it is not worth saying there,
perhaps we should not have to do so here, either.

We should have _somewhere_ that lists these pitfalls that the users
may want to be aware of, that is common to all the features that
ends up using and relying on the rename machinery.  I do not think
merge strategies documentation is the place.



^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH v2 00/10] Documentation updates: merge-strategies
  2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
                   ` (10 preceding siblings ...)
  2021-08-03 23:06 ` [PATCH 00/10] Documentation updates: merge-strategies Johannes Schindelin
@ 2021-08-04  5:28 ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
                     ` (12 more replies)
  11 siblings, 13 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git; +Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano, Elijah Newren

I noticed while updating my switch-default-merge-strategy-to-ort submission,
that many of the changes were good documentation updates that we might want
for Git v2.33.0. So I pulled those changes out and split them into lots of
little commits so that if any parts need discussion or are objectionable, we
can just drop those from this series and apply the rest for v2.33.0.

The first 9 commits are just small documentation updates, but there is one
commit at the end that updates an error message and a code comment.

Changes since v1:

 * Multiple tweaks suggested by Eric, Dscho, and Junio
 * Removed patch 7 explaining no-renames since that probably belongs in git
   diff --no-renames instead, and this series is about merge-strategies.
 * Inserted a new patch 8 that strikes some misleading or at least
   no-longer-important text from git-rebase.txt (due changes back in late
   2006).

Elijah Newren (10):
  git-rebase.txt: correct antiquated claims about --rebase-merges
  directory-rename-detection.txt: small updates due to merge-ort
    optimizations
  Documentation: edit awkward references to `git merge-recursive`
  merge-strategies.txt: update wording for the resolve strategy
  merge-strategies.txt: do not imply using copy detection is desired
  merge-strategies.txt: avoid giving special preference to patience
    algorithm
  merge-strategies.txt: fix simple capitalization error
  git-rebase.txt: correct out-of-date and misleading text about renames
  merge-strategies.txt: add coverage of the `ort` merge strategy
  Update error message and code comment

 Documentation/git-rebase.txt                  | 27 ++++++-----
 Documentation/merge-options.txt               |  4 +-
 Documentation/merge-strategies.txt            | 48 +++++++++++--------
 .../technical/directory-rename-detection.txt  | 14 +++---
 builtin/merge.c                               |  2 +-
 sequencer.c                                   |  2 +-
 6 files changed, 55 insertions(+), 42 deletions(-)


base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1059%2Fnewren%2Fort-doc-updates-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1059/newren/ort-doc-updates-v2
Pull-Request: https://github.com/git/git/pull/1059

Range-diff vs v1:

  1:  ab2367594a3 !  1:  34352397168 git-rebase.txt: correct antiquated claims about --rebase-merges
     @@ Commit message
          2019-07-31).  However, when the limitation was lifted, the documentation
          was not updated.  Update it now.
      
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-rebase.txt ##
  2:  6b89ab8d9b1 =  2:  3fdd068231a directory-rename-detection.txt: small updates due to merge-ort optimizations
  3:  c1d056f0794 !  3:  2a38320c2be Documentation: edit awkward references to `git merge-recursive`
     @@ Commit message
      
       ## Documentation/git-rebase.txt ##
      @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
     + 
       -s <strategy>::
       --strategy=<strategy>::
     - 	Use the given merge strategy.
     +-	Use the given merge strategy.
      -	If there is no `-s` option 'git merge-recursive' is used
      -	instead.  This implies --merge.
     -+	If there is no `-s` option the `recursive` strategy is the
     -+	default. This implies --merge.
     ++	Use the given merge strategy, instead of the default
     ++	`recursive`.  This implies `--merge`.
       +
       Because 'git rebase' replays each commit from the working branch
       on top of the <upstream> branch using the given strategy, using
  4:  3989f194ba9 !  4:  e422a1bc7d4 merge-strategies.txt: update wording for the resolve strategy
     @@ Metadata
       ## Commit message ##
          merge-strategies.txt: update wording for the resolve strategy
      
     -    The resolve merge strategy was given prominent positioning in this
     -    document, being listed first since it was the default at the time the
     -    document was written.  It hasn't been the default since before Git v1.0
     -    was released, though.  Move it later in the document, near `octopus` and
     -    `ours`.
     +    It is probably helpful to cover the default merge strategy first, so
     +    move the text for the resolve strategy to later in the document.
      
          Further, the wording for "resolve" claimed that it was "considered
     -    generally safe and fast", which implies that the other strategies are
     -    not.  While such an implication may have been true in 2005 when written,
     -    it may well be that `ort` is faster today (since it does not need to
     -    recurse into all directories).  Also, since `resolve` was the default
     -    for less than a year while `recursive` has been the default for a decade
     -    and a half, I think `recursive` is more battle-tested than `resolve` is.
     -    Just strike this extraneous phrase.
     -
     -    Also, provide some quick historical context that may help users
     -    understand its purpose and place in the list of merge strategies.
     +    generally safe and fast", which might imply in some readers minds that
     +    the same is not true of other strategies.  Rather than adding this text
     +    to all the strategies, just remove it from this one.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     @@ Documentation/merge-strategies.txt: subtree[=<path>];;
      +	This can only resolve two heads (i.e. the current branch
      +	and another branch you pulled from) using a 3-way merge
      +	algorithm.  It tries to carefully detect criss-cross
     -+	merge ambiguities.  It cannot handle renames.  This was
     -+	the default merge algorithm prior to November 2005.
     ++	merge ambiguities.  It does not handle renames.
      +
       octopus::
       	This resolves cases with more than two heads, but refuses to do
  5:  5f974afe47c =  5:  b1db5fdebe5 merge-strategies.txt: do not imply using copy detection is desired
  6:  6116f4750fd !  6:  44101062e0e merge-strategies.txt: avoid giving special preference to patience algorithm
     @@ Documentation/merge-strategies.txt: theirs;;
      -	matching lines (e.g., braces from distinct functions).  Use
      -	this when the branches to be merged have diverged wildly.
      -	See also linkgit:git-diff[1] `--patience`.
     -+	Deprecated shorthand for diff-algorithm=patience.
     ++	Deprecated synonym for `diff-algorithm=patience`.
       
       diff-algorithm=[patience|minimal|histogram|myers];;
       	Use a different diff algorithm while merging, which can help
  7:  7eecf879d60 <  -:  ----------- merge-strategies.txt: explain why no-renames might be useful
  8:  010702d0841 =  7:  d1521f98dee merge-strategies.txt: fix simple capitalization error
  -:  ----------- >  8:  8978132397e git-rebase.txt: correct out-of-date and misleading text about renames
  9:  37a69fd2e0b !  9:  bc92826f7e5 Documentation: add coverage of the `ort` merge strategy
     @@ Metadata
      Author: Elijah Newren <newren@gmail.com>
      
       ## Commit message ##
     -    Documentation: add coverage of the `ort` merge strategy
     +    merge-strategies.txt: add coverage of the `ort` merge strategy
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
     - ## Documentation/git-rebase.txt ##
     -@@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below.
     - 
     - -m::
     - --merge::
     --	Use merging strategies to rebase.  When the recursive (default) merge
     --	strategy is used, this allows rebase to be aware of renames on the
     --	upstream side.  This is the default.
     -+	Use merging strategies to rebase.  When either the `recursive`
     -+	(default) or `ort` merge strategy is used, this allows rebase
     -+	to be aware of renames on the upstream side.  This is the
     -+	default.
     - +
     - Note that a rebase merge works by replaying each commit from the working
     - branch on top of the <upstream> branch.  Because of this, when a merge
     -
       ## Documentation/merge-strategies.txt ##
      @@ Documentation/merge-strategies.txt: subtree[=<path>];;
       	is prefixed (or stripped from the beginning) to make the shape of
     @@ Documentation/merge-strategies.txt: subtree[=<path>];;
      +However, it ignores three of those options: `no-renames`,
      +`patience` and `diff-algorithm`.  It always runs with rename
      +detection (it handles it much faster than `recursive` does), and
     -+it specifically uses diff-algorithm=histogram.
     ++it specifically uses `diff-algorithm=histogram`.
      +
       resolve::
       	This can only resolve two heads (i.e. the current branch
 10:  2a7169c8c1b ! 10:  4a78ac53424 Update error message and code comment
     @@ builtin/merge.c: static int try_merge_strategy(const char *strategy, struct comm
       		for (x = 0; x < xopts_nr; x++)
       			if (parse_merge_opt(&o, xopts[x]))
      -				die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
     -+				die(_("Unknown strategy option: -X%s"), xopts[x]);
     ++				die(_("unknown strategy option: -X%s"), xopts[x]);
       
       		o.branch1 = head_arg;
       		o.branch2 = merge_remote_util(remoteheads->item)->name;

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH v2 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
                     ` (11 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

When --rebase-merges was first introduced, it only worked with the
`recursive` strategy.  Some time later, it gained support for merges
using the `octopus` strategy.  The limitation of only supporting these
two strategies was documented in 25cff9f109 ("rebase -i --rebase-merges:
add a section to the man page", 2018-04-25) and lifted in e145d99347
("rebase -r: support merge strategies other than `recursive`",
2019-07-31).  However, when the limitation was lifted, the documentation
was not updated.  Update it now.

Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 55af6fd24e2..8a67227846a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -1219,12 +1219,16 @@ successful merge so that the user can edit the message.
 If a `merge` command fails for any reason other than merge conflicts (i.e.
 when the merge operation did not even start), it is rescheduled immediately.
 
-At this time, the `merge` command will *always* use the `recursive`
-merge strategy for regular merges, and `octopus` for octopus merges,
-with no way to choose a different one. To work around
-this, an `exec` command can be used to call `git merge` explicitly,
-using the fact that the labels are worktree-local refs (the ref
-`refs/rewritten/onto` would correspond to the label `onto`, for example).
+By default, the `merge` command will use the `recursive` merge
+strategy for regular merges, and `octopus` for octopus merges.  One
+can specify a default strategy for all merges using the `--strategy`
+argument when invoking rebase, or can override specific merges in the
+interactive list of commands by using an `exec` command to call `git
+merge` explicitly with a `--strategy` argument.  Note that when
+calling `git merge` explicitly like this, you can make use of the fact
+that the labels are worktree-local refs (the ref `refs/rewritten/onto`
+would correspond to the label `onto`, for example) in order to refer
+to the branches you want to merge.
 
 Note: the first command (`label onto`) labels the revision onto which
 the commits are rebased; The name `onto` is just a convention, as a nod
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
                     ` (10 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

In commit 0c4fd732f0 ("Move computation of dir_rename_count from
merge-ort to diffcore-rename", 2021-02-27), much of the logic for
computing directory renames moved into diffcore-rename.
directory-rename-detection.txt had claims that all of that logic was
found in merge-recursive.  Update the documentation.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 .../technical/directory-rename-detection.txt       | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt
index 49b83ef3cc4..029ee2cedc5 100644
--- a/Documentation/technical/directory-rename-detection.txt
+++ b/Documentation/technical/directory-rename-detection.txt
@@ -2,9 +2,9 @@ Directory rename detection
 ==========================
 
 Rename detection logic in diffcore-rename that checks for renames of
-individual files is aggregated and analyzed in merge-recursive for cases
-where combinations of renames indicate that a full directory has been
-renamed.
+individual files is also aggregated there and then analyzed in either
+merge-ort or merge-recursive for cases where combinations of renames
+indicate that a full directory has been renamed.
 
 Scope of abilities
 ------------------
@@ -88,9 +88,11 @@ directory rename detection support in:
     Folks have requested in the past that `git diff` detect directory
     renames and somehow simplify its output.  It is not clear whether this
     would be desirable or how the output should be simplified, so this was
-    simply not implemented.  Further, to implement this, directory rename
-    detection logic would need to move from merge-recursive to
-    diffcore-rename.
+    simply not implemented.  Also, while diffcore-rename has most of the
+    logic for detecting directory renames, some of the logic is still found
+    within merge-ort and merge-recursive.  Fully supporting directory
+    rename detection in diffs would require copying or moving the remaining
+    bits of logic to the diff machinery.
 
   * am
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 03/10] Documentation: edit awkward references to `git merge-recursive`
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
                     ` (9 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

A few places in the documentation referred to the "`recursive` strategy"
using the phrase "`git merge-recursive`", suggesting that it was forking
subprocesses to call a toplevel builtin.  Perhaps that was relevant to
when rebase was a shell script, but it seems like a rather indirect way
to refer to the `recursive` strategy.  Simplify the references.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt       | 5 ++---
 Documentation/merge-options.txt    | 4 ++--
 Documentation/merge-strategies.txt | 9 +++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a67227846a..c3edcb07e3e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -354,9 +354,8 @@ See also INCOMPATIBLE OPTIONS below.
 
 -s <strategy>::
 --strategy=<strategy>::
-	Use the given merge strategy.
-	If there is no `-s` option 'git merge-recursive' is used
-	instead.  This implies --merge.
+	Use the given merge strategy, instead of the default
+	`recursive`.  This implies `--merge`.
 +
 Because 'git rebase' replays each commit from the working branch
 on top of the <upstream> branch using the given strategy, using
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index eb0aabd396f..f819bd8dd68 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail.
 	Use the given merge strategy; can be supplied more than
 	once to specify them in the order they should be tried.
 	If there is no `-s` option, a built-in list of strategies
-	is used instead ('git merge-recursive' when merging a single
-	head, 'git merge-octopus' otherwise).
+	is used instead (`recursive` when merging a single head,
+	`octopus` otherwise).
 
 -X <option>::
 --strategy-option=<option>::
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 2912de706bf..5d707e952aa 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -51,10 +51,11 @@ patience;;
 	See also linkgit:git-diff[1] `--patience`.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
-	Tells 'merge-recursive' to use a different diff algorithm, which
-	can help avoid mismerges that occur due to unimportant matching
-	lines (such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Defaults to the
+	`diff.algorithm` config setting.
 
 ignore-space-change;;
 ignore-all-space;;
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (2 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
                     ` (8 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

It is probably helpful to cover the default merge strategy first, so
move the text for the resolve strategy to later in the document.

Further, the wording for "resolve" claimed that it was "considered
generally safe and fast", which might imply in some readers minds that
the same is not true of other strategies.  Rather than adding this text
to all the strategies, just remove it from this one.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 5d707e952aa..f100fad1e43 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
 can also take their own options, which can be passed by giving `-X<option>`
 arguments to `git merge` and/or `git pull`.
 
-resolve::
-	This can only resolve two heads (i.e. the current branch
-	and another branch you pulled from) using a 3-way merge
-	algorithm.  It tries to carefully detect criss-cross
-	merge ambiguities and is considered generally safe and
-	fast.
-
 recursive::
 	This can only resolve two heads using a 3-way merge
 	algorithm.  When there is more than one common
@@ -106,6 +99,12 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+resolve::
+	This can only resolve two heads (i.e. the current branch
+	and another branch you pulled from) using a 3-way merge
+	algorithm.  It tries to carefully detect criss-cross
+	merge ambiguities.  It does not handle renames.
+
 octopus::
 	This resolves cases with more than two heads, but refuses to do
 	a complex merge that needs manual resolution.  It is
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 05/10] merge-strategies.txt: do not imply using copy detection is desired
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (3 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
                     ` (7 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Stating that the recursive strategy "currently cannot make use of
detected copies" implies that this is a technical shortcoming of the
current algorithm.  I disagree with that.  I don't see how copies could
possibly be used in a sane fashion in a merge algorithm -- would we
propagate changes in one file on one side of history to each copy of
that file when merging?  That makes no sense to me.  I cannot think of
anything else that would make sense either.  Change the wording to
simply state that we ignore any copies.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index f100fad1e43..e2988124581 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -16,9 +16,9 @@ recursive::
 	causing mismerges by tests done on actual merge commits
 	taken from Linux 2.6 kernel development history.
 	Additionally this can detect and handle merges involving
-	renames, but currently cannot make use of detected
-	copies.  This is the default merge strategy when pulling
-	or merging one branch.
+	renames.  It does not make use of detected copies.  This
+	is the default merge strategy when pulling or merging one
+	branch.
 +
 The 'recursive' strategy can take the following options:
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (4 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
                     ` (6 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We already have diff-algorithm that explains why there are special diff
algorithms, so we do not need to re-explain patience.  patience exists
as its own toplevel option for historical reasons, but there's no reason
to give it special preference or document it again and suggest it's more
important than other diff algorithms, so just refer to it as a
deprecated shorthand for `diff-algorithm=patience`.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index e2988124581..b54bcf68f2d 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -37,11 +37,7 @@ theirs;;
 	no 'theirs' merge strategy to confuse this merge option with.
 
 patience;;
-	With this option, 'merge-recursive' spends a little extra time
-	to avoid mismerges that sometimes occur due to unimportant
-	matching lines (e.g., braces from distinct functions).  Use
-	this when the branches to be merged have diverged wildly.
-	See also linkgit:git-diff[1] `--patience`.
+	Deprecated synonym for `diff-algorithm=patience`.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
 	Use a different diff algorithm while merging, which can help
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 07/10] merge-strategies.txt: fix simple capitalization error
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (5 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
                     ` (5 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c3edcb07e3e..cecdcfff47a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -529,7 +529,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
 where commits can be reordered, inserted and dropped at will.
 +
 It is currently only possible to recreate the merge commits using the
-`recursive` merge strategy; Different merge strategies can be used only via
+`recursive` merge strategy; different merge strategies can be used only via
 explicit `exec git merge -s <strategy> [...]` commands.
 +
 See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (6 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04 15:50     ` Ramsay Jones
  2021-08-04  5:28   ` [PATCH v2 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
                     ` (4 subsequent siblings)
  12 siblings, 1 reply; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Commit 58634dbff8 ("rebase: Allow merge strategies to be used when
rebasing", 2006-06-21) added the --merge option to git-rebase so that
renames could be detected (at least when using the `recursive` merge
backend).  However, git-am -3 gained that same ability in commit
579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28).  As such,
the comment about being able to detect renames is not particularly
noteworthy.  Remove it.  While tweaking this description, add a quick
comment about when --merge became the default.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index cecdcfff47a..73d49ec8d91 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -340,9 +340,7 @@ See also INCOMPATIBLE OPTIONS below.
 
 -m::
 --merge::
-	Use merging strategies to rebase.  When the recursive (default) merge
-	strategy is used, this allows rebase to be aware of renames on the
-	upstream side.  This is the default.
+	Using merging strategies to rebase (default).
 +
 Note that a rebase merge works by replaying each commit from the working
 branch on top of the <upstream> branch.  Because of this, when a merge
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (7 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  5:28   ` [PATCH v2 10/10] Update error message and code comment Elijah Newren via GitGitGadget
                     ` (3 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index b54bcf68f2d..210f0f850b2 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -95,6 +95,20 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+ort::
+	This is meant as a drop-in replacement for the `recursive`
+	algorithm (as reflected in its acronym -- "Ostensibly
+	Recursive's Twin"), and will likely replace it in the future.
+	It fixes corner cases that the `recursive` strategy handles
+	suboptimally, and is significantly faster in large
+	repositories -- especially when many renames are involved.
++
+The `ort` strategy takes all the same options as `recursive`.
+However, it ignores three of those options: `no-renames`,
+`patience` and `diff-algorithm`.  It always runs with rename
+detection (it handles it much faster than `recursive` does), and
+it specifically uses `diff-algorithm=histogram`.
+
 resolve::
 	This can only resolve two heads (i.e. the current branch
 	and another branch you pulled from) using a 3-way merge
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v2 10/10] Update error message and code comment
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (8 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
@ 2021-08-04  5:28   ` Elijah Newren via GitGitGadget
  2021-08-04  6:12   ` [PATCH v2 00/10] Documentation updates: merge-strategies Junio C Hamano
                     ` (2 subsequent siblings)
  12 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04  5:28 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

There were two locations in the code that referred to 'merge-recursive'
but which were also applicable to 'merge-ort'.  Update them to more
general wording.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/merge.c | 2 +-
 sequencer.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a8a843b1f54..d7b14bf4a7f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		for (x = 0; x < xopts_nr; x++)
 			if (parse_merge_opt(&o, xopts[x]))
-				die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
+				die(_("unknown strategy option: -X%s"), xopts[x]);
 
 		o.branch1 = head_arg;
 		o.branch2 = merge_remote_util(remoteheads->item)->name;
diff --git a/sequencer.c b/sequencer.c
index 7f07cd00f3f..a4e5c43fcf2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2065,7 +2065,7 @@ static int do_pick_commit(struct repository *r,
 		/*
 		 * We do not intend to commit immediately.  We just want to
 		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
+		 * that represents the "current" state for the merge machinery
 		 * to work on.
 		 */
 		if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 61+ messages in thread

* Re: [PATCH v2 00/10] Documentation updates: merge-strategies
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (9 preceding siblings ...)
  2021-08-04  5:28   ` [PATCH v2 10/10] Update error message and code comment Elijah Newren via GitGitGadget
@ 2021-08-04  6:12   ` Junio C Hamano
  2021-08-04 14:56     ` Derrick Stolee
  2021-08-04 21:47   ` Johannes Schindelin
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
  12 siblings, 1 reply; 61+ messages in thread
From: Junio C Hamano @ 2021-08-04  6:12 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget
  Cc: git, Eric Sunshine, Johannes Schindelin, Elijah Newren

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Changes since v1:
>
>  * Multiple tweaks suggested by Eric, Dscho, and Junio
>  * Removed patch 7 explaining no-renames since that probably belongs in git
>    diff --no-renames instead, and this series is about merge-strategies.
>  * Inserted a new patch 8 that strikes some misleading or at least
>    no-longer-important text from git-rebase.txt (due changes back in late
>    2006).

Looking good.
Will queue, together with the "switch" patches on top.
Thanks.

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH v2 00/10] Documentation updates: merge-strategies
  2021-08-04  6:12   ` [PATCH v2 00/10] Documentation updates: merge-strategies Junio C Hamano
@ 2021-08-04 14:56     ` Derrick Stolee
  0 siblings, 0 replies; 61+ messages in thread
From: Derrick Stolee @ 2021-08-04 14:56 UTC (permalink / raw)
  To: Junio C Hamano, Elijah Newren via GitGitGadget
  Cc: git, Eric Sunshine, Johannes Schindelin, Elijah Newren

On 8/4/2021 2:12 AM, Junio C Hamano wrote:
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> Changes since v1:
>>
>>  * Multiple tweaks suggested by Eric, Dscho, and Junio
>>  * Removed patch 7 explaining no-renames since that probably belongs in git
>>    diff --no-renames instead, and this series is about merge-strategies.
>>  * Inserted a new patch 8 that strikes some misleading or at least
>>    no-longer-important text from git-rebase.txt (due changes back in late
>>    2006).
> 
> Looking good.
> Will queue, together with the "switch" patches on top.

Chiming in to say I like this version, too.

Thanks!
-Stolee

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames
  2021-08-04  5:28   ` [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
@ 2021-08-04 15:50     ` Ramsay Jones
  2021-08-04 17:18       ` Elijah Newren
  0 siblings, 1 reply; 61+ messages in thread
From: Ramsay Jones @ 2021-08-04 15:50 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget, git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano, Elijah Newren

Hi Elijah,

On 04/08/2021 06:28, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> Commit 58634dbff8 ("rebase: Allow merge strategies to be used when
> rebasing", 2006-06-21) added the --merge option to git-rebase so that
> renames could be detected (at least when using the `recursive` merge
> backend).  However, git-am -3 gained that same ability in commit
> 579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28).  As such,
> the comment about being able to detect renames is not particularly
> noteworthy.  Remove it.  While tweaking this description, add a quick
> comment about when --merge became the default.

The last sentence of the commit message does not seem to apply to
this patch (any more ...?).

[Awesome work on 'merge -sort', by the way!]

ATB,
Ramsay Jones

> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-rebase.txt | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index cecdcfff47a..73d49ec8d91 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -340,9 +340,7 @@ See also INCOMPATIBLE OPTIONS below.
>  
>  -m::
>  --merge::
> -	Use merging strategies to rebase.  When the recursive (default) merge
> -	strategy is used, this allows rebase to be aware of renames on the
> -	upstream side.  This is the default.
> +	Using merging strategies to rebase (default).
>  +
>  Note that a rebase merge works by replaying each commit from the working
>  branch on top of the <upstream> branch.  Because of this, when a merge
> 

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames
  2021-08-04 15:50     ` Ramsay Jones
@ 2021-08-04 17:18       ` Elijah Newren
  0 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren @ 2021-08-04 17:18 UTC (permalink / raw)
  To: Ramsay Jones
  Cc: Elijah Newren via GitGitGadget, Git Mailing List, Eric Sunshine,
	Johannes Schindelin, Junio C Hamano

On Wed, Aug 4, 2021 at 9:50 AM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
> Hi Elijah,
>
> On 04/08/2021 06:28, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
> >
> > Commit 58634dbff8 ("rebase: Allow merge strategies to be used when
> > rebasing", 2006-06-21) added the --merge option to git-rebase so that
> > renames could be detected (at least when using the `recursive` merge
> > backend).  However, git-am -3 gained that same ability in commit
> > 579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28).  As such,
> > the comment about being able to detect renames is not particularly
> > noteworthy.  Remove it.  While tweaking this description, add a quick
> > comment about when --merge became the default.
>
> The last sentence of the commit message does not seem to apply to
> this patch (any more ...?).

Doh!  Yes, you're right, that last sentence should be removed.

> [Awesome work on 'merge -sort', by the way!]

Thanks!

> ATB,
> Ramsay Jones

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH 08/10] merge-strategies.txt: fix simple capitalization error
  2021-08-03 23:32     ` Elijah Newren
@ 2021-08-04 21:42       ` Johannes Schindelin
  0 siblings, 0 replies; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-04 21:42 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, Git Mailing List

Hi Elijah,

On Tue, 3 Aug 2021, Elijah Newren wrote:

> On Tue, Aug 3, 2021 at 5:01 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Hi Elijah,
> >
> > On Tue, 3 Aug 2021, Elijah Newren via GitGitGadget wrote:
> >
> > > From: Elijah Newren <newren@gmail.com>
> > >
> > > Signed-off-by: Elijah Newren <newren@gmail.com>
> > > ---
> > >  Documentation/git-rebase.txt | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > > index 7044afba362..b4429954480 100644
> > > --- a/Documentation/git-rebase.txt
> > > +++ b/Documentation/git-rebase.txt
> > > @@ -530,7 +530,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
> > >  where commits can be reordered, inserted and dropped at will.
> > >  +
> > >  It is currently only possible to recreate the merge commits using the
> > > -`recursive` merge strategy; Different merge strategies can be used only via
> > > +`recursive` merge strategy; different merge strategies can be used only via
> >
> > I am not a native speaker, so I'm eager to learn what is the correct thing
> > to do here. In particular since I continued in lower-case after a
> > semicolon for _years_, right up until some native speaker mentioned that
> > that's only correct if I continue with an incomplete sentence. If a
> > complete sentence follows the semicolon, so the advice went, I should
> > start the sentence with an upper-case letter.
> >
> > Could you help me understand the correct rules here?
>
> First I've ever heard of it; I just presumed it was a typo.  I don't
> pretend to be an expert on such topics (and although native speakers
> may have an edge in knowing what "sounds natural", I tend to presume
> we are less likely to know the rules).  However, some quick internet
> searches suggest that different style guides differ on whether to
> follow a *colon* with a capital letter[1][2][3], but everything I can
> find suggests that *semicolons* should always be followed by a
> lowercase letter (with exceptions for proper nouns)[2][4][5][and
> others...].
>
> [1] http://www.sussex.ac.uk/informatics/punctuation/colonandsemi/colon
> [2] https://www.quora.com/Why-is-capitalization-necessary-after-a-semi-colon
> [3] https://writingcenter.unc.edu/tips-and-tools/semi-colons-colons-and-dashes/
> [4] https://writeanswers.royalroads.ca/faq/199268
> [5] https://wordcounter.net/blog/2016/06/24/101812_capitalize-words-colon-semi-colon.html
>
> So, after reading through the above links, I'm fairly convinced that
> 'different' should be lowercase here.

Thank you for the detailed answer,
Dscho

^ permalink raw reply	[flat|nested] 61+ messages in thread

* Re: [PATCH v2 00/10] Documentation updates: merge-strategies
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (10 preceding siblings ...)
  2021-08-04  6:12   ` [PATCH v2 00/10] Documentation updates: merge-strategies Junio C Hamano
@ 2021-08-04 21:47   ` Johannes Schindelin
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
  12 siblings, 0 replies; 61+ messages in thread
From: Johannes Schindelin @ 2021-08-04 21:47 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget
  Cc: git, Eric Sunshine, Junio C Hamano, Elijah Newren

Hi Elijah,

On Wed, 4 Aug 2021, Elijah Newren via GitGitGadget wrote:

> Range-diff vs v1:
> [...]

This looks good to me.

Thank you for making this a very pleasant review,
Dscho

^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH v3 00/10] Documentation updates: merge-strategies
  2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
                     ` (11 preceding siblings ...)
  2021-08-04 21:47   ` Johannes Schindelin
@ 2021-08-04 23:50   ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
                       ` (9 more replies)
  12 siblings, 10 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren

I noticed while updating my switch-default-merge-strategy-to-ort submission,
that many of the changes were good documentation updates that we might want
for Git v2.33.0. So I pulled those changes out and split them into lots of
little commits so that if any parts need discussion or are objectionable, we
can just drop those from this series and apply the rest for v2.33.0.

The first 9 commits are just small documentation updates, but there is one
commit at the end that updates an error message and a code comment.

Changes since v1:

 * Multiple tweaks suggested by Eric, Dscho, and Junio
 * Removed patch 7 explaining no-renames since that probably belongs in git
   diff --no-renames instead, and this series is about merge-strategies.
 * Inserted a new patch 8 that strikes some misleading or at least
   no-longer-important text from git-rebase.txt (due changes back in late
   2006).

Changes since v2:

 * Removed sentence from commit message of patch 8 referring to a change in
   v1 that was since removed.
 * Added Stolee's and Dscho's Acked-bys.

Elijah Newren (10):
  git-rebase.txt: correct antiquated claims about --rebase-merges
  directory-rename-detection.txt: small updates due to merge-ort
    optimizations
  Documentation: edit awkward references to `git merge-recursive`
  merge-strategies.txt: update wording for the resolve strategy
  merge-strategies.txt: do not imply using copy detection is desired
  merge-strategies.txt: avoid giving special preference to patience
    algorithm
  merge-strategies.txt: fix simple capitalization error
  git-rebase.txt: correct out-of-date and misleading text about renames
  merge-strategies.txt: add coverage of the `ort` merge strategy
  Update error message and code comment

 Documentation/git-rebase.txt                  | 27 ++++++-----
 Documentation/merge-options.txt               |  4 +-
 Documentation/merge-strategies.txt            | 48 +++++++++++--------
 .../technical/directory-rename-detection.txt  | 14 +++---
 builtin/merge.c                               |  2 +-
 sequencer.c                                   |  2 +-
 6 files changed, 55 insertions(+), 42 deletions(-)


base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1059%2Fnewren%2Fort-doc-updates-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1059/newren/ort-doc-updates-v3
Pull-Request: https://github.com/git/git/pull/1059

Range-diff vs v2:

  1:  34352397168 !  1:  75b81598a80 git-rebase.txt: correct antiquated claims about --rebase-merges
     @@ Commit message
          was not updated.  Update it now.
      
          Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-rebase.txt ##
  2:  3fdd068231a !  2:  69fa233483c directory-rename-detection.txt: small updates due to merge-ort optimizations
     @@ Commit message
          directory-rename-detection.txt had claims that all of that logic was
          found in merge-recursive.  Update the documentation.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/technical/directory-rename-detection.txt ##
  3:  2a38320c2be !  3:  48f72d7e028 Documentation: edit awkward references to `git merge-recursive`
     @@ Commit message
          when rebase was a shell script, but it seems like a rather indirect way
          to refer to the `recursive` strategy.  Simplify the references.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-rebase.txt ##
  4:  e422a1bc7d4 !  4:  81a3092b9b0 merge-strategies.txt: update wording for the resolve strategy
     @@ Commit message
          the same is not true of other strategies.  Rather than adding this text
          to all the strategies, just remove it from this one.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/merge-strategies.txt ##
  5:  b1db5fdebe5 !  5:  1d144757a2e merge-strategies.txt: do not imply using copy detection is desired
     @@ Commit message
          anything else that would make sense either.  Change the wording to
          simply state that we ignore any copies.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/merge-strategies.txt ##
  6:  44101062e0e !  6:  a8381a89065 merge-strategies.txt: avoid giving special preference to patience algorithm
     @@ Commit message
          important than other diff algorithms, so just refer to it as a
          deprecated shorthand for `diff-algorithm=patience`.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/merge-strategies.txt ##
  7:  d1521f98dee !  7:  2c82aacbcbd merge-strategies.txt: fix simple capitalization error
     @@ Metadata
       ## Commit message ##
          merge-strategies.txt: fix simple capitalization error
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-rebase.txt ##
  8:  8978132397e !  8:  032dcf7c18e git-rebase.txt: correct out-of-date and misleading text about renames
     @@ Commit message
          backend).  However, git-am -3 gained that same ability in commit
          579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28).  As such,
          the comment about being able to detect renames is not particularly
     -    noteworthy.  Remove it.  While tweaking this description, add a quick
     -    comment about when --merge became the default.
     +    noteworthy.  Remove it.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/git-rebase.txt ##
  9:  bc92826f7e5 !  9:  9ae77dbc291 merge-strategies.txt: add coverage of the `ort` merge strategy
     @@ Metadata
       ## Commit message ##
          merge-strategies.txt: add coverage of the `ort` merge strategy
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## Documentation/merge-strategies.txt ##
 10:  4a78ac53424 ! 10:  0b881131b2b Update error message and code comment
     @@ Commit message
          but which were also applicable to 'merge-ort'.  Update them to more
          general wording.
      
     +    Acked-by: Derrick Stolee <dstolee@microsoft.com>
     +    Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/merge.c ##

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 61+ messages in thread

* [PATCH v3 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
                       ` (8 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

When --rebase-merges was first introduced, it only worked with the
`recursive` strategy.  Some time later, it gained support for merges
using the `octopus` strategy.  The limitation of only supporting these
two strategies was documented in 25cff9f109 ("rebase -i --rebase-merges:
add a section to the man page", 2018-04-25) and lifted in e145d99347
("rebase -r: support merge strategies other than `recursive`",
2019-07-31).  However, when the limitation was lifted, the documentation
was not updated.  Update it now.

Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 55af6fd24e2..8a67227846a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -1219,12 +1219,16 @@ successful merge so that the user can edit the message.
 If a `merge` command fails for any reason other than merge conflicts (i.e.
 when the merge operation did not even start), it is rescheduled immediately.
 
-At this time, the `merge` command will *always* use the `recursive`
-merge strategy for regular merges, and `octopus` for octopus merges,
-with no way to choose a different one. To work around
-this, an `exec` command can be used to call `git merge` explicitly,
-using the fact that the labels are worktree-local refs (the ref
-`refs/rewritten/onto` would correspond to the label `onto`, for example).
+By default, the `merge` command will use the `recursive` merge
+strategy for regular merges, and `octopus` for octopus merges.  One
+can specify a default strategy for all merges using the `--strategy`
+argument when invoking rebase, or can override specific merges in the
+interactive list of commands by using an `exec` command to call `git
+merge` explicitly with a `--strategy` argument.  Note that when
+calling `git merge` explicitly like this, you can make use of the fact
+that the labels are worktree-local refs (the ref `refs/rewritten/onto`
+would correspond to the label `onto`, for example) in order to refer
+to the branches you want to merge.
 
 Note: the first command (`label onto`) labels the revision onto which
 the commits are rebased; The name `onto` is just a convention, as a nod
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
                       ` (7 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

In commit 0c4fd732f0 ("Move computation of dir_rename_count from
merge-ort to diffcore-rename", 2021-02-27), much of the logic for
computing directory renames moved into diffcore-rename.
directory-rename-detection.txt had claims that all of that logic was
found in merge-recursive.  Update the documentation.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 .../technical/directory-rename-detection.txt       | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/technical/directory-rename-detection.txt b/Documentation/technical/directory-rename-detection.txt
index 49b83ef3cc4..029ee2cedc5 100644
--- a/Documentation/technical/directory-rename-detection.txt
+++ b/Documentation/technical/directory-rename-detection.txt
@@ -2,9 +2,9 @@ Directory rename detection
 ==========================
 
 Rename detection logic in diffcore-rename that checks for renames of
-individual files is aggregated and analyzed in merge-recursive for cases
-where combinations of renames indicate that a full directory has been
-renamed.
+individual files is also aggregated there and then analyzed in either
+merge-ort or merge-recursive for cases where combinations of renames
+indicate that a full directory has been renamed.
 
 Scope of abilities
 ------------------
@@ -88,9 +88,11 @@ directory rename detection support in:
     Folks have requested in the past that `git diff` detect directory
     renames and somehow simplify its output.  It is not clear whether this
     would be desirable or how the output should be simplified, so this was
-    simply not implemented.  Further, to implement this, directory rename
-    detection logic would need to move from merge-recursive to
-    diffcore-rename.
+    simply not implemented.  Also, while diffcore-rename has most of the
+    logic for detecting directory renames, some of the logic is still found
+    within merge-ort and merge-recursive.  Fully supporting directory
+    rename detection in diffs would require copying or moving the remaining
+    bits of logic to the diff machinery.
 
   * am
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 03/10] Documentation: edit awkward references to `git merge-recursive`
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
                       ` (6 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

A few places in the documentation referred to the "`recursive` strategy"
using the phrase "`git merge-recursive`", suggesting that it was forking
subprocesses to call a toplevel builtin.  Perhaps that was relevant to
when rebase was a shell script, but it seems like a rather indirect way
to refer to the `recursive` strategy.  Simplify the references.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt       | 5 ++---
 Documentation/merge-options.txt    | 4 ++--
 Documentation/merge-strategies.txt | 9 +++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a67227846a..c3edcb07e3e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -354,9 +354,8 @@ See also INCOMPATIBLE OPTIONS below.
 
 -s <strategy>::
 --strategy=<strategy>::
-	Use the given merge strategy.
-	If there is no `-s` option 'git merge-recursive' is used
-	instead.  This implies --merge.
+	Use the given merge strategy, instead of the default
+	`recursive`.  This implies `--merge`.
 +
 Because 'git rebase' replays each commit from the working branch
 on top of the <upstream> branch using the given strategy, using
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index eb0aabd396f..f819bd8dd68 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -112,8 +112,8 @@ With --squash, --commit is not allowed, and will fail.
 	Use the given merge strategy; can be supplied more than
 	once to specify them in the order they should be tried.
 	If there is no `-s` option, a built-in list of strategies
-	is used instead ('git merge-recursive' when merging a single
-	head, 'git merge-octopus' otherwise).
+	is used instead (`recursive` when merging a single head,
+	`octopus` otherwise).
 
 -X <option>::
 --strategy-option=<option>::
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 2912de706bf..5d707e952aa 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -51,10 +51,11 @@ patience;;
 	See also linkgit:git-diff[1] `--patience`.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
-	Tells 'merge-recursive' to use a different diff algorithm, which
-	can help avoid mismerges that occur due to unimportant matching
-	lines (such as braces from distinct functions).  See also
-	linkgit:git-diff[1] `--diff-algorithm`.
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.  Defaults to the
+	`diff.algorithm` config setting.
 
 ignore-space-change;;
 ignore-all-space;;
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 04/10] merge-strategies.txt: update wording for the resolve strategy
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (2 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
                       ` (5 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

It is probably helpful to cover the default merge strategy first, so
move the text for the resolve strategy to later in the document.

Further, the wording for "resolve" claimed that it was "considered
generally safe and fast", which might imply in some readers minds that
the same is not true of other strategies.  Rather than adding this text
to all the strategies, just remove it from this one.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 5d707e952aa..f100fad1e43 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -6,13 +6,6 @@ backend 'merge strategies' to be chosen with `-s` option.  Some strategies
 can also take their own options, which can be passed by giving `-X<option>`
 arguments to `git merge` and/or `git pull`.
 
-resolve::
-	This can only resolve two heads (i.e. the current branch
-	and another branch you pulled from) using a 3-way merge
-	algorithm.  It tries to carefully detect criss-cross
-	merge ambiguities and is considered generally safe and
-	fast.
-
 recursive::
 	This can only resolve two heads using a 3-way merge
 	algorithm.  When there is more than one common
@@ -106,6 +99,12 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+resolve::
+	This can only resolve two heads (i.e. the current branch
+	and another branch you pulled from) using a 3-way merge
+	algorithm.  It tries to carefully detect criss-cross
+	merge ambiguities.  It does not handle renames.
+
 octopus::
 	This resolves cases with more than two heads, but refuses to do
 	a complex merge that needs manual resolution.  It is
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 05/10] merge-strategies.txt: do not imply using copy detection is desired
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (3 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
                       ` (4 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Stating that the recursive strategy "currently cannot make use of
detected copies" implies that this is a technical shortcoming of the
current algorithm.  I disagree with that.  I don't see how copies could
possibly be used in a sane fashion in a merge algorithm -- would we
propagate changes in one file on one side of history to each copy of
that file when merging?  That makes no sense to me.  I cannot think of
anything else that would make sense either.  Change the wording to
simply state that we ignore any copies.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index f100fad1e43..e2988124581 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -16,9 +16,9 @@ recursive::
 	causing mismerges by tests done on actual merge commits
 	taken from Linux 2.6 kernel development history.
 	Additionally this can detect and handle merges involving
-	renames, but currently cannot make use of detected
-	copies.  This is the default merge strategy when pulling
-	or merging one branch.
+	renames.  It does not make use of detected copies.  This
+	is the default merge strategy when pulling or merging one
+	branch.
 +
 The 'recursive' strategy can take the following options:
 
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (4 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
                       ` (3 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

We already have diff-algorithm that explains why there are special diff
algorithms, so we do not need to re-explain patience.  patience exists
as its own toplevel option for historical reasons, but there's no reason
to give it special preference or document it again and suggest it's more
important than other diff algorithms, so just refer to it as a
deprecated shorthand for `diff-algorithm=patience`.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index e2988124581..b54bcf68f2d 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -37,11 +37,7 @@ theirs;;
 	no 'theirs' merge strategy to confuse this merge option with.
 
 patience;;
-	With this option, 'merge-recursive' spends a little extra time
-	to avoid mismerges that sometimes occur due to unimportant
-	matching lines (e.g., braces from distinct functions).  Use
-	this when the branches to be merged have diverged wildly.
-	See also linkgit:git-diff[1] `--patience`.
+	Deprecated synonym for `diff-algorithm=patience`.
 
 diff-algorithm=[patience|minimal|histogram|myers];;
 	Use a different diff algorithm while merging, which can help
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 07/10] merge-strategies.txt: fix simple capitalization error
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (5 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
                       ` (2 subsequent siblings)
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index c3edcb07e3e..cecdcfff47a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -529,7 +529,7 @@ The `--rebase-merges` mode is similar in spirit to the deprecated
 where commits can be reordered, inserted and dropped at will.
 +
 It is currently only possible to recreate the merge commits using the
-`recursive` merge strategy; Different merge strategies can be used only via
+`recursive` merge strategy; different merge strategies can be used only via
 explicit `exec git merge -s <strategy> [...]` commands.
 +
 See also REBASING MERGES and INCOMPATIBLE OPTIONS below.
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 08/10] git-rebase.txt: correct out-of-date and misleading text about renames
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (6 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 10/10] Update error message and code comment Elijah Newren via GitGitGadget
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Commit 58634dbff8 ("rebase: Allow merge strategies to be used when
rebasing", 2006-06-21) added the --merge option to git-rebase so that
renames could be detected (at least when using the `recursive` merge
backend).  However, git-am -3 gained that same ability in commit
579c9bb198 ("Use merge-recursive in git-am -3.", 2006-12-28).  As such,
the comment about being able to detect renames is not particularly
noteworthy.  Remove it.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-rebase.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index cecdcfff47a..73d49ec8d91 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -340,9 +340,7 @@ See also INCOMPATIBLE OPTIONS below.
 
 -m::
 --merge::
-	Use merging strategies to rebase.  When the recursive (default) merge
-	strategy is used, this allows rebase to be aware of renames on the
-	upstream side.  This is the default.
+	Using merging strategies to rebase (default).
 +
 Note that a rebase merge works by replaying each commit from the working
 branch on top of the <upstream> branch.  Because of this, when a merge
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (7 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  2021-08-04 23:50     ` [PATCH v3 10/10] Update error message and code comment Elijah Newren via GitGitGadget
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/merge-strategies.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index b54bcf68f2d..210f0f850b2 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -95,6 +95,20 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+ort::
+	This is meant as a drop-in replacement for the `recursive`
+	algorithm (as reflected in its acronym -- "Ostensibly
+	Recursive's Twin"), and will likely replace it in the future.
+	It fixes corner cases that the `recursive` strategy handles
+	suboptimally, and is significantly faster in large
+	repositories -- especially when many renames are involved.
++
+The `ort` strategy takes all the same options as `recursive`.
+However, it ignores three of those options: `no-renames`,
+`patience` and `diff-algorithm`.  It always runs with rename
+detection (it handles it much faster than `recursive` does), and
+it specifically uses `diff-algorithm=histogram`.
+
 resolve::
 	This can only resolve two heads (i.e. the current branch
 	and another branch you pulled from) using a 3-way merge
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 61+ messages in thread

* [PATCH v3 10/10] Update error message and code comment
  2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
                       ` (8 preceding siblings ...)
  2021-08-04 23:50     ` [PATCH v3 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
@ 2021-08-04 23:50     ` Elijah Newren via GitGitGadget
  9 siblings, 0 replies; 61+ messages in thread
From: Elijah Newren via GitGitGadget @ 2021-08-04 23:50 UTC (permalink / raw)
  To: git
  Cc: Eric Sunshine, Johannes Schindelin, Junio C Hamano,
	Derrick Stolee, Ramsay Jones, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

There were two locations in the code that referred to 'merge-recursive'
but which were also applicable to 'merge-ort'.  Update them to more
general wording.

Acked-by: Derrick Stolee <dstolee@microsoft.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/merge.c | 2 +-
 sequencer.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a8a843b1f54..d7b14bf4a7f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -738,7 +738,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		for (x = 0; x < xopts_nr; x++)
 			if (parse_merge_opt(&o, xopts[x]))
-				die(_("Unknown option for merge-recursive: -X%s"), xopts[x]);
+				die(_("unknown strategy option: -X%s"), xopts[x]);
 
 		o.branch1 = head_arg;
 		o.branch2 = merge_remote_util(remoteheads->item)->name;
diff --git a/sequencer.c b/sequencer.c
index 7f07cd00f3f..a4e5c43fcf2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2065,7 +2065,7 @@ static int do_pick_commit(struct repository *r,
 		/*
 		 * We do not intend to commit immediately.  We just want to
 		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
+		 * that represents the "current" state for the merge machinery
 		 * to work on.
 		 */
 		if (write_index_as_tree(&head, r->index, r->index_file, 0, NULL))
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 61+ messages in thread

end of thread, other threads:[~2021-08-04 23:51 UTC | newest]

Thread overview: 61+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03 15:35 [PATCH 00/10] Documentation updates: merge-strategies Elijah Newren via GitGitGadget
2021-08-03 15:35 ` [PATCH 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
2021-08-03 22:53   ` Johannes Schindelin
2021-08-03 15:35 ` [PATCH 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
2021-08-04  0:06   ` Junio C Hamano
2021-08-03 15:35 ` [PATCH 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
2021-08-04  0:14   ` Junio C Hamano
2021-08-04  0:19     ` Elijah Newren
2021-08-03 15:35 ` [PATCH 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
2021-08-04  0:19   ` Junio C Hamano
2021-08-04  0:37     ` Elijah Newren
2021-08-04  2:01       ` Junio C Hamano
2021-08-03 15:35 ` [PATCH 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
2021-08-03 22:56   ` Johannes Schindelin
2021-08-04  0:21   ` Junio C Hamano
2021-08-03 15:35 ` [PATCH 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
2021-08-03 17:00   ` Eric Sunshine
2021-08-03 15:35 ` [PATCH 07/10] merge-strategies.txt: explain why no-renames might be useful Elijah Newren via GitGitGadget
2021-08-04  0:28   ` Junio C Hamano
2021-08-04  0:44     ` Elijah Newren
2021-08-04  2:05       ` Junio C Hamano
2021-08-03 15:35 ` [PATCH 08/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
2021-08-03 23:01   ` Johannes Schindelin
2021-08-03 23:32     ` Elijah Newren
2021-08-04 21:42       ` Johannes Schindelin
2021-08-03 15:35 ` [PATCH 09/10] Documentation: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
2021-08-03 23:03   ` Johannes Schindelin
2021-08-03 23:43     ` Elijah Newren
2021-08-04  0:33   ` Junio C Hamano
2021-08-04  0:39     ` Elijah Newren
2021-08-03 15:35 ` [PATCH 10/10] Update error message and code comment Elijah Newren via GitGitGadget
2021-08-03 23:05   ` Johannes Schindelin
2021-08-03 23:49     ` Elijah Newren
2021-08-03 23:06 ` [PATCH 00/10] Documentation updates: merge-strategies Johannes Schindelin
2021-08-04  5:28 ` [PATCH v2 " Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
2021-08-04 15:50     ` Ramsay Jones
2021-08-04 17:18       ` Elijah Newren
2021-08-04  5:28   ` [PATCH v2 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
2021-08-04  5:28   ` [PATCH v2 10/10] Update error message and code comment Elijah Newren via GitGitGadget
2021-08-04  6:12   ` [PATCH v2 00/10] Documentation updates: merge-strategies Junio C Hamano
2021-08-04 14:56     ` Derrick Stolee
2021-08-04 21:47   ` Johannes Schindelin
2021-08-04 23:50   ` [PATCH v3 " Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 01/10] git-rebase.txt: correct antiquated claims about --rebase-merges Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 02/10] directory-rename-detection.txt: small updates due to merge-ort optimizations Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 03/10] Documentation: edit awkward references to `git merge-recursive` Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 04/10] merge-strategies.txt: update wording for the resolve strategy Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 05/10] merge-strategies.txt: do not imply using copy detection is desired Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 06/10] merge-strategies.txt: avoid giving special preference to patience algorithm Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 07/10] merge-strategies.txt: fix simple capitalization error Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 08/10] git-rebase.txt: correct out-of-date and misleading text about renames Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 09/10] merge-strategies.txt: add coverage of the `ort` merge strategy Elijah Newren via GitGitGadget
2021-08-04 23:50     ` [PATCH v3 10/10] Update error message and code comment Elijah Newren via GitGitGadget

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.