sageserpent-open / kineticMerge

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

Ignore whitespace when merging. #7

Closed sageserpent-open closed 8 months ago

sageserpent-open commented 1 year ago

Given the prevalence of automatic formatting tools, it is inevitable that a merge will be complicated by trivial whitespace changes mixed in with non-trivial ones.

Kinetic Merge should ignore whitespace changes when looking for code motion, although for the purposes of this story, it will still include the effects of whitespace changes in the final merges it performs.

In essence, runs of contiguous whitespace characters are treated as a single virtual whitespace token when looking for code motion in either branch.

sageserpent-open commented 8 months ago

As of commit SHA: bc8c93e03a3d0be5b3ef7beae060ba08820b6d61, this is already implemented for the simple token-level merge, but there are no tests to prove that it is and works properly.

Nor is there any explicit mention of the precedence that a standard Git merge would use.

Git's merge actually treats all whitespace changes as significant, unless using the ort strategy with an option to ignore whitespace switched on. Now as that as the business of ticket #9, let's not bother here.

String constants have been swept under the carpet though; whitespace is important for strings, but that is currently not the case in Kinetic Merge, this needs to be fixed.

As a work-in-progress, let's assume that a Java string constant is a good enough approximation to string constants in Java, C#, Python etc - a ticket can always be raised to refine this with language-specific tweaks... So a Java string constant will be treated as a token in its own right with internal significant whitespace.

sageserpent-open commented 8 months ago

Added tests to TokenTest plus an invariant check on Token.Whitespace, that should be enough for now.