mbox series

[0/7] Make diff3 the default conflict style

Message ID 20210609192842.696646-1-felipe.contreras@gmail.com (mailing list archive)
Headers show
Series Make diff3 the default conflict style | expand

Message

Felipe Contreras June 9, 2021, 7:28 p.m. UTC
This patch series turned out much more complicated that simply flipping
the switch and dealing with the consequences. Apparently some commands
are completely ignoring the configuration (notes and merge-tree), and
others are handling it wrong (checkout).

So in preparation I created a new test to make sure these rowdy
commands handle the configuration correctly, and then step by step I fix
them.

Once all the commands are fixed I proceed to cleanup xdiff-interface in
preparation for the switch.

And finally once the switch is flipped the documnetation is updated, and
funch of test scripts receive a temporary configuration that returns
them to the old "merge" (diff2) behavior so they pass with minimum
changes.

I have already written patches to update the tests so no configuration
is needed and they parse the diff3 style directly, but the series is
already quite verbose as it is.

One salient thorn is that from my point of view merge_recursive_config()
is implemtend wrongly and thus can't be called as other configuration
functions, like git_diff_basic_config(). It seems there's a huge area of
opportunity there to clean all that up, but that's for another series.

Felipe Contreras (7):
  test: add merge style config test
  merge-tree: fix merge.conflictstyle handling
  notes: fix merge.conflictstyle handling
  checkout: fix merge.conflictstyle handling
  xdiff: rename XDL_MERGE_STYLE_DIFF3
  xdiff: simplify style assignments
  xdiff: make diff3 the default conflictStyle

 Documentation/config/merge.txt           |  12 +--
 Documentation/git-merge-file.txt         |   2 +
 Documentation/git-merge.txt              |  28 ++----
 Documentation/git-rerere.txt             |   2 +-
 Documentation/gitattributes.txt          |   6 +-
 Documentation/technical/rerere.txt       |   3 +-
 Documentation/user-manual.txt            |   6 +-
 builtin/merge-file.c                     |   5 +-
 builtin/merge-recursive.c                |   3 +
 builtin/merge-tree.c                     |   4 +
 builtin/merge.c                          |   4 +
 builtin/notes.c                          |   3 +-
 ll-merge.c                               |   3 +-
 merge-recursive.c                        |   2 +-
 sequencer.c                              |   5 +
 t/t2023-checkout-m.sh                    |   2 +
 t/t3310-notes-merge-manual-resolve.sh    |   2 +
 t/t3311-notes-merge-fanout.sh            |   2 +
 t/t3404-rebase-interactive.sh            |   2 +
 t/t3507-cherry-pick-conflict.sh          |   2 +
 t/t4017-diff-retval.sh                   |   2 +
 t/t4048-diff-combined-binary.sh          |   2 +
 t/t4200-rerere.sh                        |   2 +
 t/t4300-merge-tree.sh                    |   2 +
 t/t6402-merge-rename.sh                  |   2 +
 t/t6403-merge-file.sh                    |   2 +
 t/t6404-recursive-merge.sh               |   2 +
 t/t6416-recursive-corner-cases.sh        |   2 +
 t/t6417-merge-ours-theirs.sh             |   2 +
 t/t6418-merge-text-auto.sh               |   2 +
 t/t6422-merge-rename-corner-cases.sh     |   2 +
 t/t6423-merge-rename-directories.sh      |   1 +
 t/t6428-merge-conflicts-sparse.sh        |   1 +
 t/t6432-merge-recursive-space-options.sh |   2 +
 t/t6440-config-conflict-markers.sh       | 123 +++++++++++++++++++++++
 t/t7201-co.sh                            |   2 +
 t/t7506-status-submodule.sh              |   1 +
 xdiff-interface.c                        |   6 +-
 xdiff/xdiff.h                            |   3 +-
 xdiff/xmerge.c                           |   4 +-
 40 files changed, 217 insertions(+), 46 deletions(-)
 create mode 100755 t/t6440-config-conflict-markers.sh

Comments

Phillip Wood June 17, 2021, 5:40 p.m. UTC | #1
On 09/06/2021 20:28, Felipe Contreras wrote:
> This patch series turned out much more complicated that simply flipping
> the switch and dealing with the consequences. Apparently some commands
> are completely ignoring the configuration (notes and merge-tree), and
> others are handling it wrong (checkout).
> 
> So in preparation I created a new test to make sure these rowdy
> commands handle the configuration correctly, and then step by step I fix
> them.
> 
> Once all the commands are fixed I proceed to cleanup xdiff-interface in
> preparation for the switch.
> 
> And finally once the switch is flipped the documnetation is updated, and
> funch of test scripts receive a temporary configuration that returns
> them to the old "merge" (diff2) behavior so they pass with minimum
> changes.
> 
> I have already written patches to update the tests so no configuration
> is needed and they parse the diff3 style directly, but the series is
> already quite verbose as it is.
> 
> One salient thorn is that from my point of view merge_recursive_config()
> is implemtend wrongly and thus can't be called as other configuration
> functions, like git_diff_basic_config(). It seems there's a huge area of
> opportunity there to clean all that up, but that's for another series.

Regrettably I shall not be commenting further on these or any future 
patches from you. I do not feel it would be a productive use of my time 
as you have not entered into the kind of constructive discussion that is 
the expected norm on this list.

Best Wishes

Phillip

> Felipe Contreras (7):
>    test: add merge style config test
>    merge-tree: fix merge.conflictstyle handling
>    notes: fix merge.conflictstyle handling
>    checkout: fix merge.conflictstyle handling
>    xdiff: rename XDL_MERGE_STYLE_DIFF3
>    xdiff: simplify style assignments
>    xdiff: make diff3 the default conflictStyle
> 
>   Documentation/config/merge.txt           |  12 +--
>   Documentation/git-merge-file.txt         |   2 +
>   Documentation/git-merge.txt              |  28 ++----
>   Documentation/git-rerere.txt             |   2 +-
>   Documentation/gitattributes.txt          |   6 +-
>   Documentation/technical/rerere.txt       |   3 +-
>   Documentation/user-manual.txt            |   6 +-
>   builtin/merge-file.c                     |   5 +-
>   builtin/merge-recursive.c                |   3 +
>   builtin/merge-tree.c                     |   4 +
>   builtin/merge.c                          |   4 +
>   builtin/notes.c                          |   3 +-
>   ll-merge.c                               |   3 +-
>   merge-recursive.c                        |   2 +-
>   sequencer.c                              |   5 +
>   t/t2023-checkout-m.sh                    |   2 +
>   t/t3310-notes-merge-manual-resolve.sh    |   2 +
>   t/t3311-notes-merge-fanout.sh            |   2 +
>   t/t3404-rebase-interactive.sh            |   2 +
>   t/t3507-cherry-pick-conflict.sh          |   2 +
>   t/t4017-diff-retval.sh                   |   2 +
>   t/t4048-diff-combined-binary.sh          |   2 +
>   t/t4200-rerere.sh                        |   2 +
>   t/t4300-merge-tree.sh                    |   2 +
>   t/t6402-merge-rename.sh                  |   2 +
>   t/t6403-merge-file.sh                    |   2 +
>   t/t6404-recursive-merge.sh               |   2 +
>   t/t6416-recursive-corner-cases.sh        |   2 +
>   t/t6417-merge-ours-theirs.sh             |   2 +
>   t/t6418-merge-text-auto.sh               |   2 +
>   t/t6422-merge-rename-corner-cases.sh     |   2 +
>   t/t6423-merge-rename-directories.sh      |   1 +
>   t/t6428-merge-conflicts-sparse.sh        |   1 +
>   t/t6432-merge-recursive-space-options.sh |   2 +
>   t/t6440-config-conflict-markers.sh       | 123 +++++++++++++++++++++++
>   t/t7201-co.sh                            |   2 +
>   t/t7506-status-submodule.sh              |   1 +
>   xdiff-interface.c                        |   6 +-
>   xdiff/xdiff.h                            |   3 +-
>   xdiff/xmerge.c                           |   4 +-
>   40 files changed, 217 insertions(+), 46 deletions(-)
>   create mode 100755 t/t6440-config-conflict-markers.sh
>
Felipe Contreras June 17, 2021, 6:24 p.m. UTC | #2
Phillip Wood wrote:
> Regrettably I shall not be commenting further on these or any future 
> patches from you. I do not feel it would be a productive use of my time 
> as you have not entered into the kind of constructive discussion that is 
> the expected norm on this list.

Please let me know where exactly I have ignored your feedback or engaged
in any kind of unconstructive discussion with you.

Without any actual proof I don't think the above is an accurate
assessment.

Also, in my opinion it's bad manners to say "I don't like you and I'm
going to mute you". Just mute.

Cheers.