search

radicle-dev / radicle-git

Everything Radicle growing around Git
Other
41 stars 5 forks source link

radicle-surf: change BlobContent to borrow its content from git2::Blob #99

Closed keepsimple1 closed 1 year ago

keepsimple1 commented 1 year ago

This patch is triggered by https://github.com/radicle-dev/radicle-git/issues/86, but later morphed into a different one. I think it's okay to keep a type BlobContent, but I wanted to avoid always copying the content bytes that could be potentially quite large.

FintanH commented 1 year ago

My worry here is that BlobContent needs to live as long as the Repository that created it. But maybe that's a fair requirement.

keepsimple1 commented 1 year ago

My worry here is that BlobContent needs to live as long as the Repository that created it. But maybe that's a fair requirement.

I also have this concern on the back of my mind. Maybe this is an optimization that we don't need yet. I'll think about it a bit more.

keepsimple1 commented 1 year ago

I have added a BlobVec to support Blob::to_owned(). Now our repo API would return a Blob<BlobRef<'a>> first, and if the user needed, they can call .to_owned() to get an owned content bytes. I hope this can address our concern. Let me know what you think.

keepsimple1 commented 1 year ago

I've rebased this PR to the latest main.

keepsimple1 commented 1 year ago

please let me know if any blockers for landing this. @FintanH