search

sageserpent-open / kineticMerge

Merge a heavily refactored codebase and stay sane.
MIT License
9 stars 2 forks source link

When overlapping sections occur, what should the user do about it? #46

Open sageserpent-open opened 1 month ago

sageserpent-open commented 1 month ago

Saw this when using Kinetic Merge on its own repository, merging to branches to make Git commit SHA: e1554c515b0b66f9c56147bb06c832b85f2f11fe.

Just running Kinetic Merge to merge the two parent histories with its defaults yields overlapping sections; there are also plenty of these seen when running CodeMotionAnalysisTest, eg:

com.sageserpent.kineticmerge.core.MappedContentSources$OverlappingSections: Overlapping section detected on side: left at path: 8, Section(label = "left", path = 8, startOffset = 79, size = 5, content = Vector(2, 5, 1, 1, 3)) (content: Vector(2, 5, 1, 1, 3)) overlaps with start of section: Section(label = "left", path = 8, startOffset = 82, size = 5, content = Vector(1, 3, 8, 3, 4)) (content: Vector(1, 3, 8, 3, 4)), overlap content: Vector(1, 3).

Now, the diagnostic issued when running the full Kinetic Merge application renders the sections in the nicer way that reflects the underlying text, but either way, the end user is presented with a message that says "It failed - over to you...".

Could we provide some intelligent hint about how to fix this?

I ended up increasing the minimum match size to workaround the overlaps and it worked - perhaps a minimum size hint can be provided? Not sure if it is worth trying to deduce whether this would guarantee lack of overlaps, especially if there might be several lots of overlaps, possibly at different match sizes...

sageserpent-open commented 1 month ago

While we're at it, there is an invariant set up by CodeMotionAnalysis.of that sections may only overlap if they share the same match size.

The overlap detection knows nothing about this and will report the first overlap it sees, which might not be the one with the largest match size (we want to fix that one as a priority if we increase the minimum match size).

Perhaps CodeMotionAnalysis.of should be draconian and throw out all matches that refer to at least one overlapping section. Worth a spike to see whether this can be delivered without killing performance and what the resulting merges would look like. If this looks good, this ticket can be changed over in favour of that...