Re: [PATCH] merge-file: warn for implicit 'myers' algorithm

[Junio C Hamano <gitster@pobox.com>]

Antonin Delpeuch <antonin@delpeuch.eu> writes:

> In the documentation of the recursive merge strategy (for instance in
> "man git-merge"), it is claimed that "recursive defaults to the
> diff.algorithm config setting". As far as I can tell, both from my
> reading of the code and my interactive testing, this is wrong. This
> affects the "merge", "rebase" and "pull" commands, which all three
> mention this configuration variable in their man page without respecting
> it. Ouch!

Thanks for digging.

> I have looked for all commands which mention diff.algorithm in their man
> page and checked whether they indeed respect it. The "diff-index",
> "diff-tree" and "diff-files" commands also make this erroneous claim.

It is not erroneous if we say that these 3 diff plumbing commands
ignore the configuration variable.  They should ignore end-user
configuration for reproducibility.

See also my earlier response to Elijah <xmqqed873vgn.fsf@gitster.g>
on a related topic.

> The --diff-algorithm CLI option (as well as the --histogram and
> siblings) are respected, but not the diff.algorithm config variable.

Then they are behaving exactly as designed, which is good.  We still
need to correct their documentation, though.

> Those inconsistencies seem to be caused by the inclusion of the
> `diff-options.txt` file in the man pages, which leads their man pages to
> documenting a bunch of config variables which are in fact ignored.

That is quite understandable mistake ;-) "git merge" and other
end-user facing commands should be taught to pay attention to both
the command line option and the configuration variable.  The
plumbing commands should pay attention to the command line only.

> In any case, I would of course make sure the "ort" strategy continues to
> ignore diff.algorithm for now, given its current default value.

It may make the effort easy to follow if you do this step-wise:

 (1) start with "all Porcelain commands pay attention to the same
     diff.algorithm variable.  The plumbing commands ignore
     diff.algorithm.  All commands, either Porcelain or plumbing,
     may have different default when unconfigured (like ort does)".

 (2) then add "each Porcelain command <cmd> pays attention to
     diff.<cmd>.algorithm if defined, otherwise diff.algorithm is
     used as a fallback default.  There is no diff.<cmd>.algorithm
     for plumbing commands---they are designed not to be affected by
     the configuration variables".

 (3) optionally, doing "all Porcelain commands, when not configured,
     will use the same default (ort is no longer special---everybody
     falls back to algorithm X)" may be desiable for consistency and
     simplicity, but it would probably want further discussion can
     be left outside of the topic (e.g. right now the best candidate
     for X may be histogram, but is it suitable for all commands?
     should this extend to plumbing, making diff-index for example
     to use X as the default not myers when the command line does
     not specify --diff-algorithm?)

Thanks.