Add a associated function to surf::diff::Diff that allows us to generate source::commit::Stats over two commits

[sebastinez]

Hello everyone,

So I wanted to propose here, that we should add an associated function to surf::diff::Diff that would allow us to generate a source::commit::Stats.

Currently it is possible to generate a surf::diff::Diff through e.g.

radicle-surf/surf/src/vcs/git.rs

Line 543 in 77b1905

pub fn diff(&self, from: Oid, to: Oid) -> Result<Diff, Error> {

But we don't have any function that would calculate the amount of additions and deletions that happened between those commits.

I'm thinking we should be able to convert the additions, deletions loops from here

radicle-surf/source/src/commit.rs

Line 135 in 77b1905

pub fn commit(browser: &mut Browser<'_>, sha1: git2::Oid) -> Result<Commit, Error> {

into the mentioned associated function.

[FintanH]

Ya, that makes sense to me. So we would actually move the source::commit::Stats type into surf::diff. I'm wondering if it would make sense to calculate the Stats while building the Diff, so it just becomes associated data rather than an associated computation. The proposed function would just be an accessor then :)

[sebastinez]

I'm wondering if it would make sense to calculate the Stats while building the Diff.

Yeah that sounds great to me 👍 Would provide a PR for that

[FintanH]

Thanks, @sebastinez! Appreciate the suggestion and the follow-up 😊

[sebastinez]

I'm wondering if it would make sense to calculate the Stats while building the Diff, so it just becomes associated data rather than an associated computation.

I did some more thinking maybe if we leave it as an associated computation, one could opt in if needed.
Also I like the structure of the surf::diff::Diff and wouldn't change it.

[FintanH]

I did some more thinking maybe if we leave it as an associated computation, one could opt in if needed.

What's the con of not having it?

I would think it's cheap to carry in the diff itself as a sub-struct since it's only two u64's. Is there something I'm missing?

[cloudhead]

Yeah since it's really cheap I don't think it's so useful to be able to opt-out?