Use FEP-c16b: Formatting MFM functions #823

Merged

floatingghost merged 7 commits from ilja/akkoma:use_fep-c16b_formatting_mfm_functions into develop 2025-02-27 12:03:23 +00:00

Conversation 5 Commits 7 Files changed 6 +58 -4

ilja commented 2024-08-04 09:50:45 +00:00

Contributor

Copy link

See https://akkoma.dev/AkkomaGang/akkoma/issues/381

We can't use the HTML content as-is. FEP-c16b was written to have a "scrubber friendly" way of representing MFM functions in HTML. Here we add support in the backend for the functions Foundkey supports.

This currently uses the parser from https://codeberg.org/ilja/mfm_parser/ but can be changed to the one under the Akkoma namespace once AkkomaGang/mfm-parser#2 is merged.

Caveats:

• FEP-c16b also has an optional discovery mechanism, this is not implemented yet (federate the new htmlMfm term, and use the content when that term is provided on incoming federation).
  • I plan to add this in a separate PR after this gets merged. This PR is already a strict improvement over the current situation, so it makes sense to me to keep this PR smaller and do the discovery in a separate PR.

Broader scope where this PR is part of:

• FEP-c16b can be seen at https://codeberg.org/fediverse/fep/src/branch/main/fep/c16b/fep-c16b.md
• The MfmParser needs big adaptions. See https://akkoma.dev/AkkomaGang/mfm-parser/pulls/2
• Akkoma front-end needs changes, see https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/410

Extra:

I fixed the earmark version to 1.4.46 bc there's apparently a bug in 1.4.47 (which I first pulled in). See my comment #823 (comment)

See <https://akkoma.dev/AkkomaGang/akkoma/issues/381> We can't use the HTML content as-is. [FEP-c16b](https://codeberg.org/fediverse/fep/src/branch/main/fep/c16b/fep-c16b.md) was written to have a "scrubber friendly" way of representing MFM functions in HTML. Here we add support in the backend for the functions Foundkey supports. This currently uses the parser from https://codeberg.org/ilja/mfm_parser/ but can be changed to the one under the Akkoma namespace once https://akkoma.dev/AkkomaGang/mfm-parser/pulls/2 is merged. Caveats: - FEP-c16b also has an optional discovery mechanism, this is not implemented yet (federate the new `htmlMfm` term, and use the `content` when that term is provided on incoming federation). - I plan to add this in a separate PR after this gets merged. This PR is already a strict improvement over the current situation, so it makes sense to me to keep this PR smaller and do the discovery in a separate PR. Broader scope where this PR is part of: - FEP-c16b can be seen at <https://codeberg.org/fediverse/fep/src/branch/main/fep/c16b/fep-c16b.md> - The MfmParser needs big adaptions. See <https://akkoma.dev/AkkomaGang/mfm-parser/pulls/2> - Akkoma front-end needs changes, see <https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/410> Extra: I fixed the earmark version to 1.4.46 bc there's apparently a bug in 1.4.47 (which I first pulled in). See my comment https://akkoma.dev/AkkomaGang/akkoma/pulls/823#issuecomment-12826

ilja referenced this pull request from AkkomaGang/akkoma-fe 2024-08-04 09:51:07 +00:00

Use FEP-c16b: Formatting MFM functions #410

ilja referenced this pull request from AkkomaGang/mfm-parser 2024-08-10 18:06:46 +00:00

Use FEP-c16b: Formatting MFM functions #2

ilja force-pushed use_fep-c16b_formatting_mfm_functions from 5fd6771bc6 to f6422cb370 2024-08-10 18:23:23 +00:00 Compare

ilja changed title from WIP: Use FEP-c16b: Formatting MFM functions to Use FEP-c16b: Formatting MFM functions 2024-08-10 18:35:52 +00:00

ilja commented 2024-08-10 18:44:06 +00:00

Author

Contributor

Copy link

I notice mix.lock also changed a bunch of stuff. I'm not used to changing dependencies, so I'm unsure if that's OK or what I should do with that?

I was even wondering why this is not ignored, since it's rebuild anyhow, but apparently the good practice is to keep it https://elixirforum.com/t/should-mix-lock-be-part-of-the-public-repository-and-package/45203

I notice mix.lock also changed a bunch of stuff. I'm not used to changing dependencies, so I'm unsure if that's OK or what I should do with that? I was even wondering why this is not ignored, since it's rebuild anyhow, but apparently the good practice is to keep it <https://elixirforum.com/t/should-mix-lock-be-part-of-the-public-repository-and-package/45203>

ilja changed title from Use FEP-c16b: Formatting MFM functions to WIP: Use FEP-c16b: Formatting MFM functions 2024-08-11 12:49:29 +00:00

ilja commented 2024-08-11 12:50:03 +00:00

Author

Contributor

Copy link

EDIT: Yes, it worked by downgrading earmark back to 1.4.46. There's an open issue for it https://github.com/pragdave/earmark/issues/361

You can ignore the below text 🤐

---

I put it in WIP again bc I just realised I forgot to run the test. One is failing bc the MFM markup changed, I already fixed that one locally. But the other one seems a problem with Earmark... Maybe bc I drew in a newer version than what the CI previously had? I'll see if I can figure it out.

Afaict, Earmark removes everything before the comment (Formatter.markdown_to_html()). Then the comment (I assume correctly) gets removed later with Formatter.html_escape("text/html").

This is the failure

  1) test format_input/3 with markdown raw HTML (Pleroma.Web.CommonAPI.UtilsTest)
     test/pleroma/web/common_api/utils_test.exs:213
     Assertion with == failed
     code:  assert result == ~s"<a href=\"http://example.org/\">OwO</a>"
     left:  ""
     right: "<a href=\"http://example.org/\">OwO</a>"
     stacktrace:
       test/pleroma/web/common_api/utils_test.exs:216: (test)

Here is the code responsible for this

alias Pleroma.Formatter

  def format_input(text, "text/markdown", options) do
    text
    |> Formatter.mentions_escape(options)
    |> Formatter.markdown_to_html()
    |> Formatter.linkify(options)
    |> Formatter.html_escape("text/html")
  end

  def markdown_to_html(text, opts \\ %{}) do
    Earmark.as_html!(text, %Earmark.Options{compact_output: true} |> Map.merge(opts))
  end

**EDIT:** Yes, it worked by downgrading earmark back to 1.4.46. There's an open issue for it https://github.com/pragdave/earmark/issues/361 You can ignore the below text 🤐 *** I put it in WIP again bc I just realised I forgot to run the test. One is failing bc the MFM markup changed, I already fixed that one locally. But the other one seems a problem with Earmark... Maybe bc I drew in a newer version than what the CI previously had? I'll see if I can figure it out. Afaict, Earmark removes everything before the comment (`Formatter.markdown_to_html()`). Then the comment (I assume correctly) gets removed later with `Formatter.html_escape("text/html")`. This is the failure ``` 1) test format_input/3 with markdown raw HTML (Pleroma.Web.CommonAPI.UtilsTest) test/pleroma/web/common_api/utils_test.exs:213 Assertion with == failed code: assert result == ~s"<a href=\"http://example.org/\">OwO</a>" left: "" right: "<a href=\"http://example.org/\">OwO</a>" stacktrace: test/pleroma/web/common_api/utils_test.exs:216: (test) ``` Here is the code responsible for this ``` alias Pleroma.Formatter def format_input(text, "text/markdown", options) do text |> Formatter.mentions_escape(options) |> Formatter.markdown_to_html() |> Formatter.linkify(options) |> Formatter.html_escape("text/html") end ``` ``` def markdown_to_html(text, opts \\ %{}) do Earmark.as_html!(text, %Earmark.Options{compact_output: true} |> Map.merge(opts)) end ```

ilja added 1 commit 2024-08-11 13:02:15 +00:00

Fix tests ... 90adb3cff5

There was one test who used MFM and now failed due to the new representation. This is now adapted so it doesn't fail any more.

There was another test failing, but I don't see how this could have been affected by the MFM changes...
But I did draw in newer dependencies, so I thought maybe a newer EARMARK dependency was now failing, and indeed.
By explicitly asking for 1.4.46 (according to mix.lock the version it was before), it now works again.

This is what was failing. It seems that earmark 1.4.47 removed everything before the comment, which it should not do.

  1) test format_input/3 with markdown raw HTML (Pleroma.Web.CommonAPI.UtilsTest)
     test/pleroma/web/common_api/utils_test.exs:213
     Assertion with == failed
     code:  assert result == ~s"<a href=\"http://example.org/\">OwO</a>"
     left:  ""
     right: "<a href=\"http://example.org/\">OwO</a>"
     stacktrace:
       test/pleroma/web/common_api/utils_test.exs:216: (test)

ilja changed title from WIP: Use FEP-c16b: Formatting MFM functions to Use FEP-c16b: Formatting MFM functions 2024-08-11 13:05:57 +00:00

ilja referenced this pull request 2024-08-18 09:54:01 +00:00

[bug] Upstream tracking issue: Bug in EarmarkParser causes newlines to not show in MFM posts when the post uses html tags #829

floatingghost commented 2024-08-20 10:35:23 +00:00

Owner

Copy link

I notice mix.lock also changed a bunch of stuff. I'm not used to changing dependencies, so I'm unsure if that's OK or what I should do with that?

yeah that's fine that's just mix updating stuff it's fine

this looks very cool, i'll test it out a bit~

>I notice mix.lock also changed a bunch of stuff. I'm not used to changing dependencies, so I'm unsure if that's OK or what I should do with that? yeah that's fine that's just mix updating stuff it's fine this looks very cool, i'll test it out a bit~

🎉 1

ilja added 1 commit 2024-12-01 10:44:25 +00:00

Merge branch 'develop' of https://akkoma.dev/AkkomaGang/akkoma into use_fep-c16b_formatting_mfm_functions d5e9f6be47

ilja force-pushed use_fep-c16b_formatting_mfm_functions from 8508d10c65 to f646e78b48 2024-12-01 10:51:45 +00:00 Compare

ilja added 1 commit 2024-12-02 11:36:08 +00:00

Add "FEP-c16b: Formatting MFM functions" to CHANGELOG.md 2624258cfd

ilja added 1 commit 2025-01-12 07:00:09 +00:00

Merge branch 'develop' of https://akkoma.dev/AkkomaGang/akkoma into use_fep-c16b_formatting_mfm_functions d56165c71e

ilja added 1 commit 2025-02-23 09:31:28 +00:00

Merge branch 'develop' of https://akkoma.dev/AkkomaGang/akkoma into use_fep-c16b_formatting_mfm_functions dce07f05d9

floatingghost commented 2025-02-27 12:03:17 +00:00

Owner

Copy link

i was running this for 70 bazillion years and forgot to merge

thanks a lot! (frfr)

i was running this for 70 bazillion years and forgot to merge thanks a lot! (frfr)

❤️ 1

floatingghost merged commit d7dd34f263 into develop 2025-02-27 12:03:23 +00:00

floatingghost deleted branch use_fep-c16b_formatting_mfm_functions 2025-02-27 12:03:23 +00:00

floatingghost referenced this pull request from a commit 2025-02-27 12:03:24 +00:00

Merge pull request 'Use FEP-c16b: Formatting MFM functions' (#823) from ilja/akkoma:use_fep-c16b_formatting_mfm_functions into develop

Oneric commented 2025-02-28 15:49:01 +00:00

Owner

Copy link

@floatingghost: the parser PR was merged too, but mix.exs still points to the fork on develop; probably want to change that

@floatingghost: the parser PR was merged too, but mix.exs still points to the fork on develop; probably want to change that

ilja referenced this pull request 2025-03-03 19:35:11 +00:00

[bug] See if we can use the content for posts, also when it's MFM. #381

ilja referenced this pull request 2025-03-09 19:32:41 +00:00

Add htmlMfm key when relevant #878

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

approved, awaiting change

  

bug

Something is not working

  

cannot reproduce

  

configuration

This requires config changes

  

documentation

This is about human-readable docs

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

extremely low priority

  

feature request

Something new?

  

Fix it yourself

Support has been attempted

  

help wanted

Need some help

  

invalid

Something is wrong

  

mastodon_api

  

needs change/feedback

Issue or PR is stalled on response from the author or a third party

  

needs docs

  

needs tests

  

not a bug

  

not our bug

Bug in a dependency of ours (meaning we can't just fix it), or in another AP implementation or client interfacing with Akkoma (which needs to be fixed on their end)

  

planned

  

pleroma_api

  

privacy

  

question

More information is needed

  

static_fe

  

triage

  

wontfix

This won't be fixed

No labels

approved, awaiting change

bug

cannot reproduce

configuration

documentation

duplicate

enhancement

extremely low priority

feature request

Fix it yourself

help wanted

invalid

mastodon_api

needs change/feedback

needs docs

needs tests

not a bug

not our bug

planned

pleroma_api

privacy

question

static_fe

triage

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

AkkomaGang/akkoma!823

No description provided.