Add @mention support to messaging (compose autocomplete, tappable render, self-mention notification)#2044
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
RCGV1
left a comment
There was a problem hiding this comment.
I found a compile-blocker plus two mention handling issues that look worth fixing before this is ready. Details inline.
| } else { | ||
| VStack(spacing: 0) { | ||
| if let query = mentionQuery { | ||
| MentionAutocomplete(query: query) { user in |
There was a problem hiding this comment.
This PR currently does not compile. Running xcodebuild test -project Meshtastic.xcodeproj -scheme Meshtastic -destination 'platform=iOS Simulator,name=iPhone 17' -only-testing:MeshtasticTests/MentionParserTests fails with Cannot find 'MentionAutocomplete' in scope and Cannot find 'MentionParser' in scope. I also checked Meshtastic.xcodeproj/project.pbxproj and it has no references to MentionAutocomplete, MentionParser, or MentionParserTests, so the new source/test files are never added to the Xcode targets. Please add these files to the Meshtastic and MeshtasticTests targets; otherwise the app target fails before the mention tests can run.
| enum MentionParser { | ||
|
|
||
| /// Pattern matching a mention token: `@!` followed by exactly 8 lowercase hex digits. | ||
| private static let mentionRegex = try! NSRegularExpression(pattern: "@!([0-9a-f]{8})") |
There was a problem hiding this comment.
This regex still matches the prefix of a longer token, e.g. @!deadbeef00 is treated as a mention of @!deadbeef, even though the documented wire format above says exactly 8 lowercase hex digits. The same prefix problem exists in containsMention via text.contains, so a malformed longer token can trigger a self-mention notification for the wrong node. Please require a non-hex/token boundary after the eighth digit, or reuse parsed bounded ranges for notification detection, and update the tooLongHex_matchesFirst8 test to reject this case.
| for (range, hexId) in ranges.reversed() { | ||
| guard let num = nodeNum(from: hexId) else { continue } | ||
| let displayName = resolveDisplayName(nodeNum: num, context: context) | ||
| let link = "[@\(displayName)](meshtastic:///nodes?nodenum=\(num))" |
There was a problem hiding this comment.
displayName comes from mesh data, so it can contain Markdown control characters such as ], [, (, ), or \. Interpolating it directly into Markdown link text can make AttributedString(markdown:) fail or create unintended Markdown/link structure for every message that mentions that node. Please escape the link text before interpolation, or build the attributed link without round-tripping through Markdown.
Implements cross-platform
@mentionfeature per meshtastic/design#21. Wire format is@!<8-hex-id>(e.g.@!deadbeef) matching Android/Web exactly.Core parser —
Meshtastic/Helpers/MentionParser.swiftactiveMentionQuery(in:)— detects in-progress@<query>at end of text; returnsnilif followed by!(resolved) or whitespace (ended)insertMentionToken(into:user:)— replaces@<partial>suffix with@!<hexId>usingInt64.toHex()resolveMentions(in:context:)— at render time, converts@!<hex>tokens to[@DisplayName](meshtastic:///nodes?nodenum=<num>)markdown links; resolves live from SwiftData — name changes reflect without re-sendingcontainsMention(of:in:)— exact wire-format self-mention check for notificationsCompose —
TextMessageField.swift+MentionAutocomplete.swiftMentionAutocompleteoverlay queries allNodeInfoEntityvia@Query, filters by long name / short name / user ID, caps at 10 results, showsCircleText+ nameTextFieldpath and the iOS 18+FormattingComposeArea(TextEditor) pathRender —
MessageText.swiftresolveMentionsbeforeAttributedString(markdown:)— mentions render as tappable underlined links showing current node display namehandleURLinterceptsmeshtastic:///nodes?nodenum=and callsappState.router.navigateToNodeDetail(nodeNum:)for deep-link navigationNotifications —
MeshPackets.swiftDocs —
docs/user/messages.md## @Mentionssection covering compose flow, tappable links, notification behaviour, and wire-format noteHow is this tested?
MeshtasticTests/MentionParserTests.swift— 23 unit tests using Swift Testing covering allMentionParserfunctions: regex matching, node-num extraction, active-query detection edge cases (resolved token, trailing space, empty string), token insertion, and self-mention detection.Screenshots/Videos (when applicable)
UI changes are in the compose field (autocomplete list on
@) and message bubbles (tappable mention links). Screenshots to be added by reviewer if needed.Checklist
docs/user/ordocs/developer/, and updated accordingly (see copilot-instructions.md for the view → doc page mapping). If no doc update is needed, add theskip-docs-checklabel.