matrix-rust-sdk icon indicating copy to clipboard operation
matrix-rust-sdk copied to clipboard

ffi: Expose filename and formatted body fields for media captions

Open SpiritCroc opened this issue 11 months ago • 3 comments

Enables clients to render media captions as per the recently merged MSC2530.

Note: Blocked on this ruma PR: https://github.com/ruma/ruma/pull/1732 - which is merged by now, but bumping ruma causes some other compile conflict right now, which should probably be addressed independently of this PR

  • [ ] Public API changes documented in changelogs (optional)

Signed-off-by: Tobias Büttner [email protected]

SpiritCroc avatar Feb 27 '24 08:02 SpiritCroc

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.84%. Comparing base (fd709b9) to head (46a5f0c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3171   +/-   ##
=======================================
  Coverage   83.84%   83.84%           
=======================================
  Files         232      232           
  Lines       24005    24005           
=======================================
+ Hits        20127    20128    +1     
+ Misses       3878     3877    -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Feb 27 '24 08:02 codecov-commenter

Hi! Thanks for the PR, would you be willing to update Ruma as well?

bnjbvr avatar Mar 08 '24 10:03 bnjbvr

Hi! Thanks for the PR, would you be willing to update Ruma as well?

Maybe I'll have a try at the weekend (I'll probably have a look at updating this PR to prepare for https://github.com/ruma/ruma/pull/1738 at some point either way), but if I remember correctly from last week, there were some compile issues with code I wasn't familiar with, so may be easier for someone else. Since I'm still somewhat of a Rust noob, I'm not planning to spend too much time on it, when it's likely that someone with better knowledge of the codebase will do that at some later point either way.

SpiritCroc avatar Mar 08 '24 11:03 SpiritCroc

Looks like someone already updated ruma, so now the PR should already compile - just rebased it onto main to let the CI test.

SpiritCroc avatar Mar 10 '24 09:03 SpiritCroc

Thanks for the PR! I would prefer to wait on https://github.com/ruma/ruma/pull/1738 to be merged before merging this one :-).

Hywan avatar Mar 11 '24 07:03 Hywan

The PR looks sensible though and should not be impacted by https://github.com/ruma/ruma/pull/1738.

Hywan avatar Mar 11 '24 07:03 Hywan