Add diff3 support#498
Conversation
|
Hi @wetneb, Sorry about the complete blackout in communications here, I've been busy with other things. I don't think there is any good reason to merge this before there's at least a POC of outputting the base revision for the structural merge, but I don't mind leaving the PR open for some time to see if anyone is interested in looking into it. On that note, I outlined some potential solutions to outputting the base revision in #496 (comment). |
|
Hi @slarse, I have a solution that is almost ready, but it surfaced a bug in jgit's own implementation of diff3: eclipse-jgit/jgit#38. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #498 +/- ##
============================================
- Coverage 82.70% 82.60% -0.10%
- Complexity 363 369 +6
============================================
Files 43 43
Lines 1769 1771 +2
Branches 303 313 +10
============================================
Hits 1463 1463
- Misses 180 182 +2
Partials 126 126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wetneb
left a comment
wetneb
left a comment
There was a problem hiding this comment.
Here is a working version, now that the fixed jgit has been released.
| val otherPred = delta.getOtherPredecessors(currentPcs).firstOrNull() | ||
| if (otherPred != null) { | ||
| remainingInconsistencies.remove(otherPred) | ||
| return nodes |
There was a problem hiding this comment.
This is quite a simplification from the original so I suspect I might have overseen something here. Curious to know what you think.
wetneb
changed the title
|
@slarse ping? |
|
@monperrus / @wetneb I don't think I will get around to integrating this. I've lost too much understanding of the internal workings of Spork over the years. I haven't written Java or Kotlin for years, either. I put a couple of hours going over this and trying to evaluate it, but I'm just not up for the task right now. Given how my open source commitment has diminished in the past years, I don't think I'll ever be. My career is taking me other places. @monperrus For the same reasons I stepped down from Spoon, I cannot put much time into maintaining Spork at this time. I can do small things, I can answer questions and provide what insights I remember, but for Spork to live on and keep growing, it needs a new maintainer. If you can staff one, feel free to treat the project as the sole property of the ASSERT group, I make no claims of ownership. But I feel I have done my part. @wetneb I'm truly sorry to have kept you waiting all this time just to get back to you with "I can't do it". It must be disappointing. Trying to integrate this change a few months back, failing, and then completely sticking my head in the sand was quite the wake-up call for me. I hope @monperrus can find someone to integrate this, otherwise you can always fork Spork (hah!) and carry on. |
|
@slarse I fully understand. thanks for all the effort you put in Spoon and Spork. |
|
thanks a lot @wetneb, that's a super useful feature. could you also document the new feature in the README? Thanks! |
4 participants
For #496.
I have added test cases (failing) with some intuitive expectations of what I would find useful as a user. I haven't thought super hard about it so I expect some cases might need adjusting.
No idea if it can be implemented, but I guess if someone wants to have a go at this, those changes should be a good start and they would only need to figure out the clever part of the algorithm.