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

Fix reflist diffs failing to compact when one of the inputs ends #1233

Closed
wants to merge 1 commit into from

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Nov 29, 2023

The previous reflist logic would early-exit the loop body if one of the lists was empty, but that skips the compacting logic entirely.

Instead of doing the early-exit, we can leave a list's ref as nil when the list end is reached and then flip the comparison result, which will essentially treat it as being greater than all others. This should preserve the general behavior without omitting the compaction.


I suspect this might fix #282 and/or #287, but I haven't tested that.

An alternate approach in the same vein is to create a "sentinel" slice like:

sentinel := []byte{255}

and then use that when we reach the end of a reflist, which avoids the need to flip the comparison result. However, the code later on that checks if the ref is set would then look like:

if pl == nil && rl[0] != sentinel[0]

which felt worse to me while also being a tad wasteful (extra slice allocation every call!). That being said, omitting the comparison flip could certainly be seen as cleaner.

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

The previous reflist logic would early-exit the loop body if one of the
lists was empty, but that skips the compacting logic entirely.

Instead of doing the early-exit, we can leave a list's ref as nil when
the list end is reached and then flip the comparison result, which will
essentially treat it as being greater than all others. This should
preserve the general behavior without omitting the compaction.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
@refi64
Copy link
Contributor Author

refi64 commented Nov 29, 2023

CI failures seem to be unrelated? Looks like r-project updated their packages:

-Success downloading https://cloud.r-project.org/bin/linux/debian/bullseye-cran40/python3-rpy2-dbgsym_3.5.10-1~bullseyecran.0_amd64.deb
-Success downloading https://cloud.r-project.org/bin/linux/debian/bullseye-cran40/python3-rpy2-dbgsym_3.5.10-1~bullseyecran.0_i386.deb
+Success downloading https://cloud.r-project.org/bin/linux/debian/bullseye-cran40/littler_0.3.18-2~bullseyecran.0_all.deb
+Success downloading https://cloud.r-project.org/bin/linux/debian/bullseye-cran40/python3-rpy2-dbgsym_3.5.12-1~bullseyecran.0_amd64.deb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aptly diff misses some packages
2 participants