sageserpent-open / kineticMerge

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

Just what is `Match.dominantElement` for anyway? #47

Closed sageserpent-open closed 4 months ago

sageserpent-open commented 4 months ago

Said method was anticipated (and for a long time, actually used) as a way of resolving priority for when a match can contribute two or three sections from the base, left and right to the output of a merge. Which one should it go for, as they are equivalent enough to constitute a match?

In the end, Resolution took over that job, but not before various tests and the migration mechanism drifted into using the dominant element as a way of modelling what makes one match different from another / the signature of the loosely-shared content of a match.

The problem is that the documentation for that method is out of date, and the notion of match state uniqueness versus a content signature is not always consistent.

Review the codebase and sort this out - keep it (and update the documentation) and / or add in a replacement technique where appropriate.

sageserpent-open commented 4 months ago

Removed Match.dominantElement and have cutover the migration implementation to use sets of matches rather than sets of dominant elements as the key to refer to a MoveDestinations, this is complete in 7264dc180d6e669cafe6e097ffac0d234c49d015.

Did some manual testing using the Amercium and Kinetic Merge repositories to see if anything is broken, both look fine.

Two things left to do:

  1. Update the design documentation. DONE
  2. Try to rationalise the use of MatchesContext.matchesFor outside the class in CodeMotionAnalysisExtension. That client class already passes in the lambda when constructing MatchesContext, so why does it need to reference it from the constructed instance? DONE
sageserpent-open commented 4 months ago

This went out in release 1.2.4, Git commit SHA: c4e9f02a20f66f8dc2eced38e8a329dc27960f8a.