Improve conflict resolution

[tguillemot]

I propose this work to solve #613: it replaces the merging approach using the rej files during an update by a two-way merge.

This PR is a based on the work of #407 with several differences:

• using two-way merge instead of three-way merge
• update the documentation to tell about how the conflict works and add an example of the check-merge-conflict pre-commit hook
• update the existing tests

Why two-way instead of three : IMHO, I think the majority of the git users are used to the two-way merge instead of the three-way. I can be easily convince to change this behaviour. Maybe we can add an option to let the user choose what he wants.

I still have some remaining questions :

1. How do we manage the retro compatibility ? The best way would be to add an option to keep the previous behaviour with warning to inform that it will be removed later.
2. What can we do with the overwrite option ?

Thanks in advance for your help and time.

[codecov-commenter]

Codecov Report

Merging #627 (5b1a817) into master (e983140) will decrease coverage by 0.23%.
The diff coverage is 90.19%.

@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
- Coverage   96.39%   96.16%   -0.24%     
==========================================
  Files          41       41              
  Lines        2722     2736      +14     
==========================================
+ Hits         2624     2631       +7     
- Misses         98      105       +7     

Flag	Coverage Δ
unittests	96.16% <90.19%> (-0.24%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tests/test_output.py	100.00% <ø> (ø)
copier/main.py	93.63% <89.13%> (-0.36%)	⬇️
tests/test_subdirectory.py	100.00% <100.00%> (ø)
tests/test_updatediff.py	100.00% <100.00%> (ø)
tests/conftest.py	84.61% <0.00%> (-7.70%)	⬇️
copier/tools.py	88.09% <0.00%> (-4.77%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e983140...5b1a817. Read the comment docs.

[yajo]

Please can you add co-autorship info with the author of #407 to credit him?

[yajo]

These lines have wrong indentation and are not valid yaml. Can you fix that please?

[yajo]

Thank you so much for this PR. It's a nice enhancement and the implementation looks much better! Regarding backwards compatibility, it has been broken in so many ways now that I don't really care that much 🤣 About overwrite... I think I somehow get the problem by looking at the code, but I'd appreciate it if you could explain it and any solutions you may have thought. Finally, please remember to add a note in the changelog.

[yajo]

BTW have you seen #562? It displays the diff interactively to accept or reject before continuing.

[tguillemot]

Please can you add co-autorship info with the author of #407 to credit him?

Of course. It's done.

I'd appreciate it if you could explain it and any solutions you may have thought

You're perfectly right, sorry for that.
In the current behaviour of copier the --overwrite option overwrite the files that already exist without asking. With the new behaviour we do not to overwrite things in the "old way", but it still make sense to give an option to the user to accept automatically merge upstream (a kind of git checkout --theirs). IMHO, I prefer to remove the overwrite option and let the user manage the conflict as he wants on his side.

[tguillemot]

BTW have you seen #562? It displays the diff interactively to accept or reject before continuing.

Indeed I missed that PR.

[yajo]

Are you fixing #461 too? 😍

[yajo]

I'll have to review more in depth, because the part you're modifying is one of the most important in copier. Just be patient please.

[tguillemot]

@yajo Don't worry for the time, I understand perfectly :)

And I have to find a way to fix windows problems first.

[bkmeneguello]

Maybe this is the perfect spot to check if the file has changed and show a brief diff to the user (difflib.unified_diff(old_upstream, new_upstream))

[barrywhart]

@yajo, @tguillemot: What's the status of this PR? If you don't have time to work on it, I can work on it.

I personally prefer 3-way merge: That's the behavior that gives you conflict markers, right? E.g.

<<<<<<< HEAD
foo
=======
bar
>>>>>>> cb1abc6bd98cfc84317f8aa95a7662815417802d

@yajo: You mentioned needing to review it carefully due to the risk of breaking copier. Should we consider making the behavior configurable?

• Old way: .rej files
• New way: Inline conflict markers
• Potentially: New way 2: 2-way merge

[yajo]

Continuing in #807, which has no conflicts