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

MSC2530: Body field as media caption #2530

Merged
merged 11 commits into from
Feb 19, 2024

Conversation

tulir
Copy link
Member

@tulir tulir commented May 7, 2020

Rendered

Implementations:

Signed-off-by: Tulir Asokan tulir@maunium.net


FCP tickyboxes

@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels May 7, 2020
@turt2live turt2live self-requested a review May 7, 2020 16:39
Comment on lines 37 to 38
name. To avoid this problem, we could require that `body` and `filename` must
be different for `body` to be interpreted as a caption.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another thing to note here is that body is required, so the filename is probably a reasonable fallback for body in the event that no description is given.

@turt2live turt2live removed their request for review November 10, 2020 05:28
MurzNN added a commit to MurzNN/matrix-doc that referenced this pull request Nov 27, 2020
MurzNN added a commit to MurzNN/matrix-doc that referenced this pull request Nov 30, 2020
MurzNN added a commit to MurzNN/matrix-doc that referenced this pull request Nov 30, 2020
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
@theotheroracle

This comment was marked as off-topic.

@MurzNN

This comment was marked as off-topic.

@turt2live
Copy link
Member

Please use comments on the diff to receive replies.

@MurzNN
Copy link
Contributor

MurzNN commented Jun 14, 2022

Yeah, sorry, and seems this MSC is ok, so I've commented a diff in the most suitable MSC here: #2529 (comment)

@turt2live turt2live moved this from Ready for FCP ticks to In FCP in Spec Core Team Backlog Feb 14, 2024
@mscbot
Copy link
Collaborator

mscbot commented Feb 19, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Feb 19, 2024
@turt2live turt2live merged commit ea24933 into matrix-org:old_master Feb 19, 2024
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Feb 19, 2024
@turt2live turt2live moved this from In FCP to Requires spec writing in Spec Core Team Backlog Feb 19, 2024
turt2live pushed a commit that referenced this pull request Feb 19, 2024
* Proposal to use body field as media caption

* Add paragraph about relation-based captions being difficult for bridges

* Clarify how to treat body when filename is not present

* Refactor proposal text

* Fix heading size

* Add problem statement

* Add links to and quotes from current spec

* Adjust wording and quote m.audio body spec

* Clarify that m.location and m.sticker are out of scope for this proposal

* Add examples and summary of changes

* Fix JSON syntax in example
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1731

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Feb 25, 2024
@turt2live turt2live moved this from Requires spec writing to Requires spec PR review in Spec Core Team Backlog Feb 25, 2024
@turt2live
Copy link
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Feb 26, 2024
@turt2live turt2live moved this from Requires spec PR review to Done to some definition in Spec Core Team Backlog Feb 26, 2024
bnjbvr pushed a commit to matrix-org/matrix-rust-sdk that referenced this pull request Mar 19, 2024
Now that there is some support for [MSC2530](matrix-org/matrix-spec-proposals#2530), I gave adding sending captions a try. ( This is my first time with Rust 😄  )

I tried it on Element X with a hardcoded caption and it seems to work well
![image](https://github.com/matrix-org/matrix-rust-sdk/assets/683652/597e5ebf-f7f2-498f-97a4-ac98613c1134)

(It even got forwarded through mautrix-whatsapp and the caption was visible on the Whatsapp side)

---

* ffi: Expose filename and formatted body fields for media captions

In relevance to MSC2530

* MSC2530: added the ability to send media with captions

Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>

* signoff

Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>

* fixing the import messup

* fix missing parameters in documentation

* fix formatting

* move optional parameters to the end

* more formatting fixes

* more formatting fixes

* rename url parameter to filename in send_attachment and helpers

* fix send_attachment documentation example

* move caption and formatted_caption into attachmentconfig

* fix formatting

* fix formatting

* fix formatting (hopefully the last one)

* updated stale comments

* simplify attachment message comments

---------

Signed-off-by: Marco Antonio Alvarez <surakin@gmail.com>
Co-authored-by: SpiritCroc <dev@spiritcroc.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Status: Done to some definition
Status: Done for now
Spec Core Team Backlog
  
Done to some definition
Development

Successfully merging this pull request may close these issues.

None yet

10 participants