Repository navigation
refa: Translation Module - #205
Merged
Merged
Conversation
新增 "Keep formatting" 设置，翻译时保留文本的粗体、斜体等格式，新增相关配置项和翻译逻辑，重构翻译代码以支持格式保留，同时优化翻译任务的生命周期管理，避免页面销毁后仍执行翻译任务。 thanks to @OctoGramApp Co-authored-by: nick <64551534+null-nick@users.noreply.github.com> Co-authored-by: waifucon <parsastdev@duck.com> Co-Authored-By: M3.1-Flash-Preview <model@minimax.io>
Reviewer's GuideRefactors third-party message, poll, and article translation around rich-text entity preservation and configurable formatting retention, while moving local translation state into MessageObject and making translation work lifecycle-scoped so it cannot outlive its chat or article viewer. Sequence diagram for rich-text message translationsequenceDiagram
participant User
participant ChatActivity
participant MessageTrans
participant MessageTransText
participant Translator
participant MessageObject
User->>ChatActivity: translateMessages()
ChatActivity->>MessageTrans: translateOne()
MessageTrans->>MessageTransText: queryTextOf(source)
alt KeepFormatting enabled
MessageTransText-->>MessageTrans: HTML with entities
else KeepFormatting disabled
MessageTransText-->>MessageTrans: Plain text
end
MessageTrans->>Translator: translate(target, query, context)
Translator-->>MessageTrans: translated text
MessageTrans->>MessageTransText: merge(source, translated, separator)
MessageTransText-->>MessageTrans: translatedText with entities
MessageTrans->>MessageObject: markTranslatedLocally()
MessageObject->>MessageObject: updateTranslation(false)
Sequence diagram for lifecycle-scoped translation cancellationsequenceDiagram
participant User
participant ChatActivity
participant TranslationScope
participant Translator
participant ArticleViewer
User->>ChatActivity: translateMessages()
ChatActivity->>TranslationScope: launch translation job
TranslationScope->>Translator: translate requests
User->>ChatActivity: onFragmentDestroy()
ChatActivity->>TranslationScope: cancelTranslations()
TranslationScope-->>Translator: cancel pending work
User->>ArticleViewer: close article
ArticleViewer->>ArticleViewer: destroyArticleViewer()
ArticleViewer->>TranslationScope: cancelArticleTranslations()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/ArticleTrans.kt" line_range="172" />
<code_context>
}
+ if (cancel.get()) return false
+
+ val translated = runCatching { Translator.translate(text) }.getOrNull()
+ progress.tick { updatePaintSize() }
+ if (translated == null) {
</code_context>
<issue_to_address>
**Cancellation shows failure dialogs**
When a translation coroutine is canceled while a translator call is suspended or an article viewer is destroyed, `runCatching` consumes `CancellationException`, and the article `catch (Exception)` also treats it as a provider failure while the separate `cancel` flag remains false. The message and article paths schedule failure dialogs; the article `finally` also schedules `updatePaintSize` after destruction, so users see UI work against a canceled or destroyed screen.
Propagate `CancellationException` through `runCatching`, handle it separately from provider failures, and skip article UI callbacks after cancellation.
Also at `TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/MessageTrans.kt:206-209`, `TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/MessageTrans.kt:238-241`, `TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/ArticleTrans.kt:129-134`.
</issue_to_address>
### Comment 2
<location path="TMessagesProj/src/main/java/tw/nekomimi/nekogram/transtale/html/HTMLKeeper.java" line_range="70-73" />
<code_context>
+
+ ArrayList<CharacterStyle> currentActiveSpans = new ArrayList<>(Arrays.asList(spans));
+
+ for (int j = lastActiveSpans.size() - 1; j >= 0; j--) {
+ CharacterStyle lastSpan = lastActiveSpans.get(j);
+ if (!currentActiveSpans.contains(lastSpan)) {
+ closeTagFor(out, lastSpan);
+ }
+ }
</code_context>
<issue_to_address>
**Overlapping formatting emits invalid HTML**
When source formatting ranges overlap without being properly nested, `toHtml` closes a span without closing and reopening still-active nested spans, producing crossing tags such as `<b>…<i>…</b>…</i>`. Providers receive malformed markup, so translated formatting is lost or misapplied.
Serialize overlapping spans by closing and reopening active spans so the output always has properly nested tags.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and a faulty translation or formatting conversion could be stored in the local translation cache and remain displayed on rebuilt messages after the code is reverted. The affected content is bounded and can be cleared or retranslated; it does not move money, change access, delete records, or create an irreversible external side effect.
Blocking findings: TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/ArticleTrans.kt:172, TMessagesProj/src/main/java/tw/nekomimi/nekogram/transtale/html/HTMLKeeper.java:73
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
6 open findings
What changed in this PR
Adds rich-text preservation and safer coroutine lifecycles to NekoX’s message and article translation systems.
Changes:
- Adds the “Keep formatting” setting and HTML/entity conversion utilities.
- Refactors message and poll translation with caching and bounded concurrency.
- Adds cancellation handling for chat and article destruction.
| File | Description |
|---|---|
| TMessagesProj/src/main/res/values/strings_na.xml | Updated as part of this pull request. |
| TMessagesProj/src/main/kotlin/xyz/nextalone/nagram/NaConfig.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/kotlin/xyz/nextalone/nagram/helper/MessageHelper.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/ui/MessageHelper.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/transtale/html/HTMLTagAttributesHandler.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/transtale/html/HTMLKeeper.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/transtale/html/CustomElementHandler.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/settings/NekoGeneralSettingsActivity.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/MessageTransText.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/MessageTransPoll.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/MessageTrans.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/java/tw/nekomimi/nekogram/parts/ArticleTrans.kt | Updated as part of this pull request. |
| TMessagesProj/src/main/java/org/telegram/ui/ChatActivity.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/org/telegram/ui/Cells/ChatMessageCell.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/org/telegram/ui/ArticleViewer.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/org/telegram/tgnet/TLRPC.java | Updated as part of this pull request. |
| TMessagesProj/src/main/java/org/telegram/messenger/MessageObject.java | Updated as part of this pull request. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+129
to
+132
| } catch (e: Exception) { | ||
| if (!cancel.get()) { | ||
| UIUtil.runOnUIThread { showArticleTransFailed(status, e) } | ||
| } |
| UIUtil.runOnUIThread { showArticleTransFailed(status, e) } | ||
| } | ||
| } finally { | ||
| UIUtil.runOnUIThread(Runnable { updatePaintSize() }) |
| }.awaitAll() | ||
| } finally { | ||
| messages.forEach { it.translating = false } | ||
| UIUtil.runOnUIThread { if (!cancel.get()) status?.uDismiss() } |
Comment on lines
+206
to
211
| val translated = runCatching { | ||
| Translator.translate(target, MessageTransText.queryTextOf(source), context) | ||
| }.getOrElse { | ||
| reportError(target, listOf(message), it, cancel, status) | ||
| return false | ||
| } |
Comment on lines
+253
to
+255
| private fun markTranslatedLocally(message: MessageObject, block: (TLRPC.Message) -> Unit) { | ||
| block(message.messageOwner) | ||
| message.translatedLocally = true |
Comment on lines
+342
to
+344
| } else if (mSpan instanceof BlockquoteSpan) { | ||
| entity = new TLRPC.TL_messageEntityBlockquote(); | ||
| entity.collapsed = true; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
新增 "Keep formatting" 设置，翻译时保留文本的粗体、斜体等格式，新增相关配置项和翻译逻辑，重构翻译代码以支持格式保留，同时优化翻译任务的生命周期管理，避免页面销毁后仍执行翻译任务。
Summary by Sourcery
Improve the translation module with formatting preservation, richer message support, and lifecycle-safe translation tasks.
New Features:
Bug Fixes:
Enhancements:
Chores: