diff --git a/CHANGELOG.md b/CHANGELOG.md index b0c47941..248103c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and the versioning follows [Semantic Versioning](https://semver.org/). ### ✨ Added +- Mentions now render as a pill instead of blending into the surrounding text, so it is obvious at a glance when someone is named — on the activity page, in the inline diff comments, in drafts, and in the PR description alike. - Proxy settings now take a list of **direct connections**: hosts that bypass the proxy and connect straight out, so an internal code platform, its git remote, or a self-hosted model stays reachable while everything else still goes through the proxy. Uses the familiar `NO_PROXY` syntax (a domain covers its subdomains), and applies to every outbound path at once — REST, git and the LLM call. - A review that fails because the model is unavailable now says so and tells you what to do — with a local CLI provider (claude / codex) the model comes from that CLI's own configuration, so it has to be changed there. diff --git a/CHANGELOG.zh-CN.md b/CHANGELOG.zh-CN.md index 22781768..55e47be0 100644 --- a/CHANGELOG.zh-CN.md +++ b/CHANGELOG.zh-CN.md @@ -9,6 +9,7 @@ ### ✨ 新增 +- @提及 改为胶囊标签展示，不再淹没在正文里，一眼即可看出点到了谁——活动页、内联 diff 评论、草稿与 PR 描述一致生效。 - 代理设置新增**直连地址**列表：列出的地址跳过代理直接连接，内网代码平台、它的 git 远端或自建模型服务因此保持可达，其余流量照常走代理。沿用通行的 `NO_PROXY` 写法（填域名同时覆盖子域），并对所有出站路径一并生效——REST、git 与 LLM 调用。 - 因模型不可用而失败的评审现在会明确说明，并给出处理方式——使用本地 CLI 供应商（claude / codex）时，模型来自该 CLI 自身的配置，需要在那里更换。 diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/PrInfoView.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/PrInfoView.tsx index b14e6ae7..14ad6074 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/PrInfoView.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/PrInfoView.tsx @@ -4,6 +4,7 @@ import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; import type { StoredPullRequest } from '@meebox/shared'; import { REMOTE_REHYPE_PLUGINS } from '../../../../lib/markdown'; +import { remarkMentions } from '../../../../lib/remark-mention'; import { formatTimestamp } from '../../../../utils/time'; import { Avatar, @@ -48,7 +49,7 @@ export function PrInfoView({ pr }: PrInfoViewProps) { {/* Bitbucket remote uses \r\n line endings; when remark parses, CR and LF each count as a line break → a single newline gets treated as a paragraph separator, adding blank space between each list item. Normalize to \n */} ) : ( + // Mentions render as pills here too: a draft is a comment the user is about to post, and the same body must + // not read differently before and after publishing (see docs/arch/01-platform/04-comment-interactions.md).
{draft.body.trim() ? ( - + {draft.body} ) : ( diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/drafts/DraftsPanel.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/drafts/DraftsPanel.tsx index a80ba10e..9d82a861 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/drafts/DraftsPanel.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/drafts/DraftsPanel.tsx @@ -8,6 +8,7 @@ import { invoke } from '../../../../../api'; import { formatBackendError } from '../../../../../errors'; import { useDraftsForPr } from '../../../../../stores/drafts-store'; import { ConfirmModal } from '../../../../common'; +import { remarkMentions } from '../../../../../lib/remark-mention'; // posted no longer exists (successful publish deletes the local draft), filters keep only publishable / all / rejected type Filter = 'all' | 'publishable' | 'rejected'; @@ -250,7 +251,11 @@ export function DraftsPanel({ pr, onJumpToAnchor, capabilities, readOnly = false
{d.body.trim() ? ( {d.body} diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/CommentMarkdown.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/CommentMarkdown.tsx index 2d6d279b..db12b8ed 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/CommentMarkdown.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/CommentMarkdown.tsx @@ -3,6 +3,7 @@ import remarkBreaks from 'remark-breaks'; import remarkGfm from 'remark-gfm'; import { REMOTE_REHYPE_PLUGINS } from '../../../../../lib/markdown'; import { remarkEmojiShortcodes } from '../../../../../lib/remark-emoji'; +import { remarkMentions } from '../../../../../lib/remark-mention'; import { transformBitbucketUrl } from '../../../../common'; /** @@ -29,8 +30,8 @@ export function CommentMarkdown({ (list: readonly T[] | null | undefined, extra: readonly T[]): T[] { return Array.from(new Set([...(list ?? []), ...extra])); @@ -44,6 +45,11 @@ const schema = { td: extend(defaultSchema.attributes?.td, ['align']), th: extend(defaultSchema.attributes?.th, ['align']), a: extend(defaultSchema.attributes?.a, ['rel', 'target']), + // The @mention pill (remark-mention) renders as , and sanitize would otherwise strip + // the class, leaving the mention indistinguishable from prose. Allowed as a **value-restricted** attribute + // (`[name, ...allowed values]`), not as free-form className: the body is user-generated, so permitting arbitrary + // classes on a span would let a comment borrow any style in the app. + span: extend(defaultSchema.attributes?.span, [['className', MENTION_CLASS]]), '*': extend(defaultSchema.attributes?.['*'], ['align']), }, }; diff --git a/apps/desktop/src/renderer/src/lib/remark-mention.ts b/apps/desktop/src/renderer/src/lib/remark-mention.ts new file mode 100644 index 00000000..fc6b15f8 --- /dev/null +++ b/apps/desktop/src/renderer/src/lib/remark-mention.ts @@ -0,0 +1,70 @@ +import { findMentions } from '@meebox/shared'; + +/** Minimal shape of an mdast node (only the fields this plugin uses, avoiding an mdast type dependency). */ +interface MdNode { + type: string; + value?: string; + children?: MdNode[]; + data?: { hName?: string; hProperties?: Record }; +} + +/** The class the rendered pill carries; must stay in sync with the sanitize allowlist in lib/markdown.ts. */ +export const MENTION_CLASS = 'comment-mention'; + +/** Split one text value into text / mention nodes, or null when it holds no mention (the common case — keep the node). */ +function splitMentions(text: string): MdNode[] | null { + const found = findMentions(text); + if (found.length === 0) return null; + const out: MdNode[] = []; + let last = 0; + for (const { start, end, name } of found) { + if (start > last) out.push({ type: 'text', value: text.slice(last, start) }); + out.push({ + type: 'mention', + // A node type mdast does not know: mdast-util-to-hast's unknown handler builds an element from the children and + // applies hName / hProperties, which is the documented way to emit custom markup from a remark plugin. + data: { hName: 'span', hProperties: { className: MENTION_CLASS } }, + // The pill shows `@name`, not the raw token: Bitbucket's quoted form (`@"first.last"`) carries quotes that are + // platform syntax, not part of anyone's name, and showing them inside the pill is noise. This is the one place + // the rendered text intentionally differs from the source — everything else is styling only. + children: [{ type: 'text', value: `@${name}` }], + }); + last = end; + } + if (last < text.length) out.push({ type: 'text', value: text.slice(last) }); + return out; +} + +/** + * remark plugin: render `@mention` tokens in a comment body as a pill instead of leaving them as running text, so a + * mention reads as "a person" at a glance rather than as prose that happens to start with `@`. + * + * Only mdast `text` nodes are rewritten, so code spans and code blocks are untouched for free — their content never + * lives in text nodes, meaning an `@Override` in a snippet or an email in a fenced block stays literal. Which runs + * count as mentions is decided by `findMentions` in shared, the reading counterpart of the `formatMention` used by the + * editors, so the two directions cannot disagree about the syntax (notably Bitbucket's quoted `@"first.last"`). + */ +export function remarkMentions() { + return (tree: MdNode): void => { + const walk = (node: MdNode): void => { + if (!node.children) return; + const out: MdNode[] = []; + let changed = false; + for (const child of node.children) { + if (child.type === 'text' && child.value?.includes('@')) { + const parts = splitMentions(child.value); + if (parts) { + out.push(...parts); + changed = true; + continue; + } + } else { + walk(child); + } + out.push(child); + } + if (changed) node.children = out; + }; + walk(tree); + }; +} diff --git a/apps/desktop/src/renderer/src/styles/common/markdown.scss b/apps/desktop/src/renderer/src/styles/common/markdown.scss index 0a9e705b..65232abe 100644 --- a/apps/desktop/src/renderer/src/styles/common/markdown.scss +++ b/apps/desktop/src/renderer/src/styles/common/markdown.scss @@ -148,3 +148,20 @@ margin-right: $space-2; } } + +// @mention pill (remark-mention → span.comment-mention). Rendered as one unit so a mention reads as "a person" rather +// than as prose that happens to start with `@`. Scoped to the whole file rather than nested inside `.markdown`, +// because the class is emitted by the markdown pipeline wherever a comment body is rendered — including the draft +// preview, whose container carries `.markdown`, and any future surface that may not. +.comment-mention { + display: inline-block; + padding: 0 5px; + border-radius: $radius-pill; + background: $color-accent-bg-fade; + color: $color-accent; + font-weight: 500; + // Keep line height from growing where a mention sits in a dense comment: the pill borrows the line box it is in. + line-height: 1.4; + // A long username must wrap with the text rather than force the comment to scroll sideways. + word-break: break-word; +} diff --git a/docs/arch/01-platform/04-comment-interactions.md b/docs/arch/01-platform/04-comment-interactions.md index 89ee0131..5c956a90 100644 --- a/docs/arch/01-platform/04-comment-interactions.md +++ b/docs/arch/01-platform/04-comment-interactions.md @@ -72,6 +72,18 @@ Both layers are a **pure convenience**: the user can still freely type any `@nam **Consistency across surfaces (design philosophy)**: every comment-interaction behavior — reactions, `@mention` (local + remote), attachments, reply / edit / delete — must be **identical on all comment surfaces**: the comments/activity page (`CommentItem`), the inline diff comment zone (`InlineCommentZone`), and the inline draft editor (`DraftZone`). This is enforced by **sharing the leaf components / hooks** (`CommentReplyEditor`, `MentionTextarea`, `useReactions`, `useCommentThread`) rather than reimplementing per surface, so a surface can only differ in layout, never in interaction behavior. When adding or changing an interaction, wire it into **all** surfaces (thread the same props down each path) — a capability reaching only one surface is a bug, not a scope choice. +### @mention rendering: a pill, not running text + +A mention is a reference to a person, and left as plain text it disappears into the sentence — exactly when the reader is scanning a thread for whether it concerns them. So a mention renders as a **pill** (`remark-mention` → `span.comment-mention`), applied wherever author-written prose that can name someone is rendered: the activity page, the inline comment zone, both draft surfaces, and the PR description. Drafts are included because a draft is a comment about to be posted, and the same body must not read differently before and after publishing; the PR description is included because it is the same kind of authored text, and a mention that is a pill in one place and plain text in another reads as a bug. Agent-facing markdown (chat, finding cards, rule previews) is **not** included — nothing there addresses a person. + +Three things worth knowing about the implementation: + +- **Parsing is the counterpart of writing, and lives beside it.** `findMentions` sits next to `formatMention` in `shared/mention.ts`, so the syntax — notably Bitbucket's quoted `@"first.last"`, which the server requires for a username containing a dot — is defined once instead of once per direction. +- **It is syntactic, not resolved.** There is no authoritative local list of who exists on the remote (a mention may name someone outside this PR's participants), so anything shaped like a mention is styled. That is the right side of the trade only because a false positive costs a tinted background and nothing else: the text is not altered and nothing becomes clickable. Boundary rules exclude the common false positives — an email address (no leading boundary before `@`), a scoped package (`@scope/pkg`), and trailing sentence punctuation. Code spans and fences are excluded for free, since only mdast `text` nodes are rewritten. +- **The class has to be allowlisted for sanitize, by value.** Comment bodies pass through `rehype-sanitize`, which strips `class` from a `span` by default; the pill class is allowed as a **value-restricted** attribute rather than as free-form `className`, so a comment cannot borrow arbitrary app styles by writing raw HTML. + +The pill is also the one place the rendered text intentionally differs from the source: it shows `@first.last`, dropping Bitbucket's quotes, which are platform syntax rather than part of anyone's name. + ### File-level comments: whole-file anchor + capability degradation A comment can anchor to a **whole file** (not a specific line) where the platform supports it (`fileLevelComments`). This is modeled by a `PrCommentAnchor` **without a `line`** (path + side only): `anchor == null` → PR summary; `anchor` with a line → inline; `anchor` without a line → file-level. The comment `kind` (`'summary' | 'inline' | 'file'`) mirrors this. diff --git a/packages/shared/src/mention.ts b/packages/shared/src/mention.ts index 2d521dc5..a0218407 100644 --- a/packages/shared/src/mention.ts +++ b/packages/shared/src/mention.ts @@ -26,3 +26,53 @@ export function formatMention(platform: PlatformKind, user: Pick + findMentions(text).map((m) => text.slice(m.start, m.end)); + +describe('formatMention', () => { + it('quotes a Bitbucket username that needs it, and leaves a simple one bare', () => { + expect(formatMention('bitbucket-server', { name: 'first.last' })).toBe('@"first.last"'); + expect(formatMention('bitbucket-server', { name: 'jdoe' })).toBe('@jdoe'); + }); + + it('uses the bare form on GitHub / GitLab', () => { + expect(formatMention('github', { name: 'jdoe' })).toBe('@jdoe'); + expect(formatMention('gitlab', { name: 'jdoe' })).toBe('@jdoe'); + }); +}); + +describe('findMentions', () => { + it('reads back both forms formatMention can write', () => { + const quoted = formatMention('bitbucket-server', { name: 'first.last' }); + const bare = formatMention('github', { name: 'jdoe' }); + expect(findMentions(`ping ${quoted} and ${bare}`).map((m) => m.name)).toEqual([ + 'first.last', + 'jdoe', + ]); + }); + + it('finds a mention at the start of the body and after an opening bracket', () => { + expect(tokens('@jdoe please look')).toEqual(['@jdoe']); + expect(tokens('(@jdoe) and [@ada]')).toEqual(['@jdoe', '@ada']); + }); + + it('ignores an email address', () => { + // The `@` has no leading boundary, so neither the local part nor the domain is a mention. + expect(tokens('mail me at user@example.com please')).toEqual([]); + }); + + it('ignores a scoped package name', () => { + expect(tokens('install @meebox/shared first')).toEqual([]); + // ...but a mention immediately before a slash-free word is still found. + expect(tokens('ask @meebox about it')).toEqual(['@meebox']); + }); + + it('leaves sentence punctuation out of the name', () => { + expect(findMentions('thanks @jdoe.')[0]?.name).toBe('jdoe'); + expect(findMentions('cc @jdoe, @ada').map((m) => m.name)).toEqual(['jdoe', 'ada']); + }); + + it('reports offsets that exactly span the token', () => { + const text = 'hi @jdoe there'; + const [m] = findMentions(text); + expect(text.slice(m!.start, m!.end)).toBe('@jdoe'); + }); + + it('handles several mentions in one run, including adjacent ones', () => { + expect(tokens('@a @b @c')).toEqual(['@a', '@b', '@c']); + }); + + it('finds nothing in text without a mention', () => { + expect(findMentions('no mentions here')).toEqual([]); + expect(findMentions('')).toEqual([]); + }); + + it('is not confused by a stray @ or an empty quoted token', () => { + expect(findMentions('a @ b')).toEqual([]); + expect(findMentions('a @"" b')).toEqual([]); + }); +});