Skip to content

Conversation

Byron
Copy link
Collaborator

@Byron Byron commented Oct 6, 2025

Unfortunately it does break CI and it appears to affect how myers works.

This didn't seem to be the issue in v0.2, and all algorithms should have the same result.

Tasks

  • port the fix
  • figure out why it's not working

@Byron
Copy link
Collaborator Author

Byron commented Oct 6, 2025

@pascalkuthe It looks like strangely enough, on 0.1 the backport is breaking Meyers, at least it's causing different results.

Screenshot 2025-10-06 at 05 40 06

Maybe it's also what's expected and Meyers was supposed to be different? Probably not, as you mentioned that this was only supposed to be a performance optimisation.

Maybe this hints as missing test coverage in v0.2? At least, I would have expected the same test to fail there. Even when using the tests/helix_syntax.rs.Myers.diff file from master, it fails. This is probably expected as the sliders are different. But maybe the sliders being different masked something else in 0.2?

Your expertise would definitely be appreciated as this is probably quite impossible for me to debug without opening a very big fass.

@pascalkuthe
Copy link
Owner

@Byron the test changes look fine to me these are just sliders. As I said before how sliders get resolved depends on the implementation (as in both results are correct). While this fix usually is just about performance it can affect which diff gets chosen when there are multiple ambigous ones.

This didn't trigger any failures on master/0.2 as there I run the postprocessing step in tests that runs the slider heuristic so no matter which position the algorithm selects for a slider the postprocessing will always patch it to the same endresult.

I think for the bacskport you can simply update the snapshots

@Byron
Copy link
Collaborator Author

Byron commented Oct 6, 2025

Thanks a lot for the suggestion!

I will do that and merge.

@Byron Byron marked this pull request as ready for review October 6, 2025 18:20
@Byron Byron merged commit 8883a8b into maintain-0.1 Oct 6, 2025
5 checks passed
@Byron Byron deleted the fix-is-odd branch October 6, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants