Repository navigation
chore(mobile): upgrade gpt_markdown to 1.3.4 and migrate to new APIs - #8184
saminsohag wants to merge 5 commits into
Conversation
🔐 Codex Security Review
|
02df67f to
2265793
Compare
loganj
left a comment
There was a problem hiding this comment.
🤖 Larry and Auto: an independent adversarial review of af14d87ba491. Source review, plus a local run at this head: just mobile-check found no issues, and just mobile-test passed (3733 passed, 16 skipped; unconfigured-push suite 4 passed). Mobile CI has not run on this fork PR.
The migration matches the real gpt_markdown 1.3.4 API (inlineLinkBuilder, LinkBuildDetails.labelSpans, LinkTextSpan.wrapping, InlinePattern.prefixed, autolinkSchemes, default scopes), and pubspec.lock is consistent. The stray-backtick fix is exact.
Why we are not approving yet: merged with current main (158b3bf56), this does not compile (inline at link_normalizer.dart:11). The fix needs a design choice for message previews, so the rebased result should get another look.
Other items:
- Question: screen-reader access to ordinary links (inline,
message_content.dart:681). - Error (small): email autolinks look tappable but do nothing (inline,
:713). - Advice: mention and channel patterns inside URLs (inline,
:402); the forum preview overflow stripe (inline). - Convention: the PR body is stale. #7545 is merged (Oct 21), so the "Depends on #7545" note no longer applies. The "pulse reply preview" it mentions is not in this diff. "Visible differences" also leaves out some 1.3.x parser changes users will see:
Textfollowed by a---line becomes a setext heading,__x__becomes bold,\*loses its backslash, HTML entities are decoded, and<!-- -->is hidden. The setext heading may surprise chat users. - Preference: gpt_markdown's parser now decodes
[,\and]itself, so_markdownLabelEntityPatternmay be redundant, and with it thelabelSpansworkaround at:594._useMessageInlineComponentsandinline_components.dartnow holdInlinePatterns, so those names are stale.
af14d87 to
f518ab7
Compare
|
Rebased onto main and addressed the review in f518ab7.
|
f518ab7 to
40a71d1
Compare
40a71d1 to
4dab1cc
Compare
Move mentions, channels and custom emoji from InlineMd/inlineComponents
to InlinePattern/inlinePatterns, and linkBuilder to inlineLinkBuilder.
This puts mobile on gpt_markdown's default parser.
Replace normalizeBareLinks with gpt_markdown's autolink
(autolinkSchemes: {'buzz'}), as the package docs recommend.
Fix what the new rendering exposed:
- the mention rewrite added a stray backtick when the count was odd
- a Buzz link chip merged its paragraph into one screen-reader stop
- height-bounded previews overflowed instead of clipping
Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com>
Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com>
A backslash inside an angle autolink is literal in CommonMark, but shielding turned `\>` into a stand-in, so <https://e.com/a\> lost its closing bracket. Leave angle autolinks untouched (doubling only authored markers). gpt_markdown 1.3.4 now restores escapes in multi-line strikethrough, so Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com> block#8925's known-limitation row expects # instead of its fullwidth form.
Links render as widgets again so a hold can open the link sheet and tint the link; gpt_markdown text spans support neither. Each ordinary link is keyed message-link:<url>, and link tests find links by that key instead of as LinkTextSpans, restoring upstream assertions where they existed. A Buzz URL of an unknown shape stays plain text instead of becoming a link whose tap and Open action do nothing, as it was before the migration. Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com>
4dab1cc to
bfaf4dd
Compare
- Resolve escapes inside link destinations and bare URLs instead of shielding them: gpt_markdown swaps inline-pattern matches for placeholders even inside URLs, so a stand-in there could not be restored and the link opened a broken address. - Wrap bare HTTP(S) URLs that gpt_markdown's autolink rejects (a port, a host without a dot, an @) in <...>, so localhost, :8443 and mastodon-style URLs link again. - Give autolink labels the themed link style, so bare URLs match authored links instead of drawing near body-text grey. - Offer Open for mailto links in the long-press sheet, matching the tap. - Render an authored [label](buzz://unknown) as plain text, not a link-styled run that does nothing. Signed-off-by: Samin Yeasar Sohag <sssohag172@gmail.com>
Summary
Upgrades mobile's
gpt_markdownfrom 1.2.1 to 1.3.4 and moves off the APIs that 1.3 deprecates, so mobile renders through the package's default parser instead of the legacy regex pipeline.API migration
@mention,#channeland custom emoji move fromInlineMd+inlineComponentstoInlinePattern+inlinePatterns. PassinginlineComponentsforced the whole widget onto the legacy parser.InlinePattern's default scopes already exclude link labels, so the explicitallScopesExceptLinkLabeloverrides from fix(mobile): keep token labels visible in authored Markdown links #7545 are no longer needed; the[#channel](url)/[@mention](url)/[:emoji:](url)regression tests still pass.linkBuilder→inlineLinkBuilder. Links stay widgets viaasWidgetSpan, so mobile: long-press a message link for open, copy and share #9135's long-press (the Open/Copy/Share link sheet and the press tint) keeps working; gpt_markdown's text-span links support neither. Tap handling moved into_openLink, unchanged in behavior.[/]itself, so_markdownLabelEntityPatternis removed.Autolinking
Buzz's own
normalizeBareLinkspre-processor is removed in favor ofgpt_markdown's autolink withautolinkSchemes: {'buzz'}, as the package docs recommend. Visible differences:@(http://localhost:3000,https://example.com:8443,https://mastodon.social/@alice) still link: gpt_markdown's bare autolink rejects them, so they are wrapped in<...>, which it links.www.hosts and email addresses become links. Email links open the mail app.buzz://URL of an unknown shape stays plain text, as before.Inbox and search previews (
MessagePreview) usednormalizeBareLinkstoo. They now recognize bare and<angle>URLs themselves, so media URLs still show as "Image"/"Video", channel links as chips, and message/repo links get their short label.Other 1.3.x parser changes
These come with the package's default parser and are visible in messages:
---line becomes a (setext) heading, as it already does on desktop (react-markdown + remark-gfm).__x__renders as bold.&are decoded.<!-- comments -->are hidden.Backslash escapes (rebased onto #8925)
#8925 landed while this was open. It replaces each backslash escape with an invisible stand-in before parsing, then restores the character when rendering. This PR keeps that, because 1.3.4's own escape handling reads
\[x\]and\(x\)as math, while #8925 (like CommonMark and desktop) shows them literally.InlineMdthat restores escapes is now anInlinePatternin all scopes, so it does not force the legacy parser.[x](https://e.com/a\_b)links tohttps://e.com/a_b): gpt_markdown swaps inline-pattern matches for placeholders even inside URLs, so a stand-in there could not be restored.inlineLinkBuilderrestores stand-ins in autolink labels.<https://e.com/a\>are left alone when escapes are swapped out. A backslash inside them is literal, and swapping it out broke the closing>.MessagePreviewuses the shared helpers inmarkdown_escapes.dartfor its bare and angle URL detection.#) no longer happens with 1.3.4; its test row now expects#.Known limitation: gpt_markdown 1.3.4 drops the backslash before
>in an angle autolink (<https://e.com/a\>links tohttps://e.com/a). CommonMark keeps it.Fixes the new rendering exposed
MessageContentre-added backticks around every odd segment, which adds one extra backtick when the count is odd (e.g. a 4-backtick fence closer). Previously hidden; with autolink the link swallowed it.[x](https://e.com/?u=@alice)or a barehttps://e.com/?q=#generalhad the token cut out of the URL. Mention and channel patterns now also skip a token right after=,&,?or%.MessageContentinto a max height; the new block layout reported aRenderFlexoverflow. It now lays the content out at its natural height and clips it.Related issue
Closes #8179.
Related:
Testing
just mobile-check— format and analyze clean, no deprecation warnings.just mobile-test— 4561 passed, 67 skipped, 0 failed.just file-size-check— passes.message_content_autolink_test.dartports the deleted normalizer's cases (code-span boundaries, delimiters, punctuation, entity permalinks) to assertions on whatMessageContentactually renders, and checks that email links open.preview_text_test.dartcovers bare and angle URLs in previews.link_normalizer_test.dartruns the code-boundary table (backtick and tilde fences, quotes, escaped backticks) againstmapMarkdownProse.message-link:<url>(chips keepbuzz-link-chip:<url>), and link tests find links by that key.just mobile-dev): bare links and emails render as links, trailing punctuation stays outside,[x](https://e.com/?u=@alice)andhttps://e.com/?q=#generalstay single links with no pills,#channelpills still render, links open in Safari, and text followed by---renders as a heading. The simulator has no Mail app, so the email tap is covered byopens email linksinstead.[\#buzz](url)shows#buzz,\[x\] \(y\)show as text (not math),\*x\* \_y\_are not emphasized,a \\ bshows one backslash, code keepsa\#b, an escaped backtick stays literal, and<https://example.com/a>opens in Safari. Search previews show the same message without backslashes or stand-ins, with no@alicepill from a link URL.buzz-review-completed