Skip to content

Improve blame ranges - #2204

Merged
Sebastian Thiel (Byron) merged 6 commits into
GitoxideLabs:mainfrom
cruessler:improve-blame-ranges
Nov 1, 2025
Merged

Sebastian Thiel (Byron) merged 6 commits into
GitoxideLabs:mainfrom
cruessler:improve-blame-ranges

Conversation

@cruessler

Copy link
Copy Markdown
Contributor

This PR is a continuation of #1976. I took the original PR by Bart Dubbeldam (@holodorum) and applied the suggestions I had given in my reviews of his PR. I also added a few tests and adapted some parts to gitoxide’s conventions.

These are the most important changes:

  • a rename from range to ranges that was done so the variable name now corresponds to the type name BlameRanges.
  • the conversion of BlameRanges into an enum, to more idiomatically cover the special case WholeFile.
  • making sure BlameRanges keeps its ranges sorted as that is important for the blame algorithm.

There’s one decision that requires particular attention. When you add a range to a WholeFile, it is turned into a PartialFile containing the given range. This mirrors the difference of git blame vs. git blame -L a,b. The initial version of the PR returned an error instead, but that makes the API less ergonomic, so I decided to make the change.

Co-authored-by: Bart Dubbeldam <bartdubbeldam2000@gmail.com>
Co-authored-by: Bart Dubbeldam <bartdubbeldam2000@gmail.com>
This modification introduces changes to the `BlameRanges` struct,
converting it into an enum to support both `PartialFile` and
`WholeFile`. Internally the ranges in `BlameRanges` are stored as
zero-based exclusive ranges now.

Co-authored-by: Bart Dubbeldam <bartdubbeldam2000@gmail.com>
Co-authored-by: Bart Dubbeldam <bartdubbeldam2000@gmail.com>
The comment was referring to previous versions of the code.
@Byron

Byron commented Oct 30, 2025

Copy link
Copy Markdown
Member

Did you say this is ready for merging and hanging on me?
Maybe I missed this completely.

@cruessler

Copy link
Copy Markdown
Contributor Author

Yes! If I remember correctly, nothing is missing from my side (though it has been a couple of weeks since I last touched this PR 😅).

@Byron Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long wait, I don't know what happened there.

Hopefully this only happens once a year or so 😅.

@Byron
Sebastian Thiel (Byron) merged commit 663b41e into GitoxideLabs:main Nov 1, 2025
29 checks passed
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