Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deal with small ambiguous matches gracefully. #31

Open
sageserpent-open opened this issue Apr 2, 2024 · 3 comments
Open

Deal with small ambiguous matches gracefully. #31

sageserpent-open opened this issue Apr 2, 2024 · 3 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@sageserpent-open
Copy link
Owner

sageserpent-open commented Apr 2, 2024

This addresses the need for Git commit SHA: 75e445a.

The minimum match size had to be jemmied up to 5 to avoid ambiguous matches of the snippet:

            @Override
            public void 

that is a prefix of several methods in the test source inputs.

The presence of ambiguous matches isn't a problem in itself, but one of the methods is deleted, along with the ambiguous prefix. This should propagate the deletion, except that because of the ambiguity, execution reaches this dead end.

The job is to rectify this - either:

  1. Find a way of propagating the deletion to just the 'right' destination.
  2. Ignore ambiguous matches when propagating (and issue a useful diagnostic suggesting that the match size should be increased).
  3. Automatically retry the merge with a larger minimal match size.

Ticket #6 addresses this too, but something expedient should be done first.

@sageserpent-open sageserpent-open added bug Something isn't working enhancement New feature or request labels Apr 2, 2024
@sageserpent-open
Copy link
Owner Author

sageserpent-open commented Apr 2, 2024

There is an underlying theme here - when there are ambiguous matches that involve two or more moved sections, how do we know what the move destinations are? We can state that any section that is part of the longest common subsequence is not a source or destination of a move, but multiple moves can cross over.

There is a similar problem when an ambiguous match involves a section that is deleted and another that is moved. Which one is which?

Common sense helps in that we can try to correlate the sections via their containing file names - this can work for simple cases, at least.

@sageserpent-open
Copy link
Owner Author

The existing implementation as of Git commit SHA: 09e28d0 does not exclude matched sections that are part of the longest common subsequence from being considered as move destinations, although it does exclude them from being treated as move sources.

Fixing that might provide a lightweight solution that could cover some of the more common problems, although it won't work with potentially crossed-over moves, or moves versus deletions.

@sageserpent-open
Copy link
Owner Author

sageserpent-open commented Apr 6, 2024

A cheap and cheerful solution is to propagate all of the conflicting changes (including a deletion) for an ambiguous match to all of the destinations. The user has the context to remove all but the pertinent change at each destination, or even to revert the change altogether.

We don't want to have to do this all the time where moves aren't crossed-over, but once the change in the previous comment is in, this might be a workable solution for the time being.

sageserpent-open added a commit that referenced this issue Apr 6, 2024
…is is intended as a quick-and-dirty fix for #31 that solves some of the problems covered by that ticket.
sageserpent-open added a commit that referenced this issue May 10, 2024
…them. This is intended as a quick-and-dirty fix for #31 that solves some of the problems covered by that ticket."

This reverts commit 1ac8a4d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant