fix: out of bounds panic when diffing large number of words #695
Conversation
JonathanxD
force-pushed
the
fix/out-of-bounds-panic
branch
4 times, most recently
from
29f8c10
to
0027838
Compare
Fixes Wilfred#688, fixes Wilfred#694, fixes Wilfred#682 and fixes Wilfred#681. This works around the fact that `str::lines` does not include last newline character. This means that `"a\n".lines().count() == "a".lines().count()`, which breaks the 1:1 assumption made by display implementations.
JonathanxD
force-pushed
the
fix/out-of-bounds-panic
branch
from
f4161f2
to
b686f58
Compare
|
I ran a small script to compare the output of db281c6 with my branch to find the cause of regression, it's all the trailing newlines that were not being printed before. |
|
Thanks for the pull request, and the clear explanation! This sounds like a regression from 8c004be. |
You're welcome.
I'm not sure if this is the cause of the regression because unless I'm missing something, the changes in this commit are just inlining the content of @Wilfred Do you have any plans to merge my changes in the near future? If so, I can update the regression file so the pipeline succeeds. If you don't want to merge, I can use the examples provided in the reported issues to bisect which commit caused the regression and bring it to the discussion. |
|
What's the status of this PR? I'm running into this issue too. |
Idk, everything is fine from my side. It depends on how @Wilfred wants to go from there. I've been personally just building difftastic with my patches on top since without them difft would crash 90% of the time for my use-case. |
I dropped all the old text because it does not make sense anymore.
The problem is basically that Rust's
str::lines()will return the same number of lines regardless of the last character being a new line or not, and the assumptions made on inline and side-by-side display implementations depends on having a clear distinction of those cases.Those changes introduces a function that will chain an additional element when the string ends with a newline. I'm not aware of any function from stdlib that does what we need (and I couldn't find one by looking at the documentation), the closest one was
split, but it has obvious implications (\r\n, for example).This does solves #688, #694, #682 and #681, I tested all the provided files and they're all working without any panics.