Use FEP-c16b: Formatting MFM functions

ilja commented

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

Contributor

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

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

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

ci/woodpecker/pr/build-amd64 Pipeline is pending

Details

ci/woodpecker/pr/build-arm64 Pipeline is pending

Details

ci/woodpecker/pr/docs Pipeline is pending

Details

ci/woodpecker/pr/lint Pipeline is pending

Details

ci/woodpecker/pr/test Pipeline is pending

Details

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

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

ci/woodpecker/pr/build-amd64 Pipeline is pending approval

Details

ci/woodpecker/pr/build-arm64 Pipeline is pending approval

Details