fix: address review findings (#2131)

This commit is contained in:
Jinjing 2026-05-16 17:14:39 -07:00 committed by GitHub
parent d603242766
commit fba16f73cc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 333 additions and 1 deletions

View File

@ -963,6 +963,51 @@
text-underline-offset: 2px;
}
/* ── Git conflict decorations ───────────────────────────────────── */
.monaco-editor .orca-conflict-marker-line {
background: color-mix(in srgb, var(--destructive) 12%, transparent);
}
.monaco-editor .orca-conflict-section-line {
border-left: 3px solid transparent;
}
.monaco-editor .orca-conflict-current-line {
background: color-mix(in srgb, var(--git-decoration-modified) 12%, transparent);
border-left-color: color-mix(in srgb, var(--git-decoration-modified) 72%, transparent);
}
.monaco-editor .orca-conflict-base-line {
background: color-mix(in srgb, var(--muted-foreground) 7%, transparent);
border-left-color: color-mix(in srgb, var(--muted-foreground) 36%, transparent);
}
.monaco-editor .orca-conflict-incoming-line {
background: color-mix(in srgb, var(--git-decoration-renamed) 11%, transparent);
border-left-color: color-mix(in srgb, var(--git-decoration-renamed) 68%, transparent);
}
.monaco-editor .orca-conflict-line-decoration {
width: 4px !important;
margin-left: 3px;
background: var(--destructive);
border-radius: 999px;
}
.monaco-editor .orca-conflict-margin {
background: color-mix(in srgb, var(--destructive) 8%, transparent);
}
.monaco-editor .orca-conflict-marker-label {
color: var(--destructive);
font-family: var(--font-sans, system-ui, sans-serif);
font-size: 11px;
font-weight: 600;
font-style: normal;
opacity: 0.92;
}
/* Why: Monaco's find-widget button tooltips (Close, Find in Selection, etc.)
render inside a `.context-view` wrapper `.workbench-hover-container`
`.workbench-hover.compact`, positioned above the target button. The wrapper's

View File

@ -163,6 +163,7 @@ export function EditorContent({
onSave={isMarkdown ? md.mdSave : handleSave}
worktreeId={activeFile.worktreeId}
markdownAnnotationsEnabled={markdownReviewToolsEnabled && isMarkdown && mdViewMode !== 'rich'}
conflictDecorationsEnabled={activeFile.conflict?.conflictStatus === 'unresolved'}
revealLine={
pendingEditorReveal?.filePath === activeFile.filePath ? pendingEditorReveal.line : undefined
}
@ -194,6 +195,12 @@ export function EditorContent({
viewMode: mdViewMode
})
if (activeFile.conflict?.conflictStatus === 'unresolved') {
// Why: conflict markers are source text the user must edit directly.
// Rich/preview markdown modes can hide or reinterpret those marker lines.
return <div className="h-full min-h-0">{renderMonacoEditor(fc)}</div>
}
// Why: the render-mode helper already folded size into the mode decision.
// Keep the explanatory banner here so the user understands why "rich" view
// currently shows Monaco instead.

View File

@ -29,6 +29,7 @@ import {
createMarkdownDocLinkDecorationController,
type MarkdownDocLinkDecorationController
} from './monaco-markdown-doc-link-decorations'
import { buildGitConflictDecorations, hasGitConflictMarkers } from './monaco-conflict-decorations'
import { findWorktreeById } from '@/store/slices/worktree-helpers'
import type { DiffComment } from '../../../../shared/types'
import { isMarkdownComment } from '@/lib/diff-comment-compat'
@ -50,6 +51,7 @@ type MonacoEditorProps = {
markdownDocuments?: MarkdownDocument[]
worktreeId?: string
markdownAnnotationsEnabled?: boolean
conflictDecorationsEnabled?: boolean
}
export default function MonacoEditor({
@ -65,7 +67,8 @@ export default function MonacoEditor({
revealMatchLength,
markdownDocuments,
worktreeId,
markdownAnnotationsEnabled = false
markdownAnnotationsEnabled = false,
conflictDecorationsEnabled = false
}: MonacoEditorProps): React.JSX.Element {
const editorRef = useRef<editor.IStandaloneCodeEditor | null>(null)
const editorContainerRef = useRef<HTMLDivElement | null>(null)
@ -74,6 +77,7 @@ export default function MonacoEditor({
const languageRef = useRef(language)
languageRef.current = language
const markdownDocLinkDecorationsRef = useRef<MarkdownDocLinkDecorationController | null>(null)
const conflictDecorationsRef = useRef<editor.IEditorDecorationsCollection | null>(null)
const revealDecorationRef = useRef<editor.IEditorDecorationsCollection | null>(null)
const revealHighlightTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
const revealRafRef = useRef<number | null>(null)
@ -332,6 +336,8 @@ export default function MonacoEditor({
})
editorInstance.onDidDispose(() => {
conflictDecorationsRef.current?.clear()
conflictDecorationsRef.current = null
editorRef.current = null
setMountedEditor(null)
setCommentPopover(null)
@ -542,6 +548,27 @@ export default function MonacoEditor({
markdownDocLinkDecorationsRef.current?.refresh()
}, [content, language])
useEffect(() => {
const ed = mountedEditor
if (!ed) {
return
}
if (!conflictDecorationsEnabled || !hasGitConflictMarkers(content)) {
conflictDecorationsRef.current?.clear()
return
}
// Why: Git conflict marker lines are ordinary file text; Monaco needs
// explicit decorations so unresolved blocks remain visible while editing.
const decorations = buildGitConflictDecorations(content)
if (!conflictDecorationsRef.current) {
conflictDecorationsRef.current = ed.createDecorationsCollection(decorations)
return
}
conflictDecorationsRef.current.set(decorations)
}, [conflictDecorationsEnabled, content, mountedEditor])
useEffect(() => {
updateMarkdownCompletionDocuments()
}, [updateMarkdownCompletionDocuments])
@ -553,6 +580,8 @@ export default function MonacoEditor({
}
markdownDocLinkDecorationsRef.current?.dispose()
markdownDocLinkDecorationsRef.current = null
conflictDecorationsRef.current?.clear()
conflictDecorationsRef.current = null
}
}, [])

View File

@ -0,0 +1,82 @@
import { describe, expect, it } from 'vitest'
import {
buildGitConflictDecorations,
findGitConflictBlocks,
hasGitConflictMarkers
} from './monaco-conflict-decorations'
describe('findGitConflictBlocks', () => {
it('finds standard conflict marker blocks', () => {
const content = [
'before',
'<<<<<<< HEAD',
'current',
'=======',
'incoming',
'>>>>>>> branch',
'after'
].join('\n')
expect(findGitConflictBlocks(content)).toEqual([
{
startLine: 2,
separatorLine: 4,
endLine: 6
}
])
})
it('keeps common ancestor markers inside diff3 conflict blocks', () => {
const content = [
'<<<<<<< HEAD',
'current',
'||||||| parent of branch',
'base',
'=======',
'incoming',
'>>>>>>> branch'
].join('\n')
expect(findGitConflictBlocks(content)).toEqual([
{
startLine: 1,
baseLine: 3,
separatorLine: 5,
endLine: 7
}
])
})
})
describe('buildGitConflictDecorations', () => {
it('builds section and marker decorations for a conflict block', () => {
const decorations = buildGitConflictDecorations(
['<<<<<<< HEAD', 'current', '=======', 'incoming', '>>>>>>> branch'].join('\n')
)
expect(decorations).toHaveLength(5)
expect(decorations[0]).toMatchObject({
range: { startLineNumber: 2, endLineNumber: 2 },
options: { className: 'orca-conflict-section-line orca-conflict-current-line' }
})
expect(decorations[1]).toMatchObject({
range: { startLineNumber: 4, endLineNumber: 4 },
options: { className: 'orca-conflict-section-line orca-conflict-incoming-line' }
})
expect(decorations[2]).toMatchObject({
range: { startLineNumber: 1, endLineNumber: 1 },
options: {
className: 'orca-conflict-marker-line',
linesDecorationsClassName: 'orca-conflict-line-decoration',
after: { content: ' Current change' }
}
})
})
it('detects incomplete markers without producing bogus ranges', () => {
const content = ['<<<<<<< HEAD', 'current'].join('\n')
expect(hasGitConflictMarkers(content)).toBe(true)
expect(buildGitConflictDecorations(content)).toEqual([])
})
})

View File

@ -0,0 +1,169 @@
import type { editor, IRange } from 'monaco-editor'
type ConflictBlock = {
startLine: number
baseLine?: number
separatorLine: number
endLine: number
}
type ConflictSection = 'current' | 'base' | 'incoming'
function isGitConflictMarkerLine(line: string): boolean {
return (
line.startsWith('<<<<<<<') ||
line.startsWith('|||||||') ||
line === '=======' ||
line.startsWith('>>>>>>>')
)
}
function getLineEndColumn(line: string): number {
return line.length + 1
}
function makeWholeLineRange(startLineNumber: number, endLineNumber: number): IRange {
return {
startLineNumber,
startColumn: 1,
endLineNumber,
endColumn: 1
}
}
function makeMarkerRange(lineNumber: number, line: string): IRange {
return {
startLineNumber: lineNumber,
startColumn: 1,
endLineNumber: lineNumber,
endColumn: getLineEndColumn(line)
}
}
function makeMarkerDecoration(
lineNumber: number,
line: string,
label: string
): editor.IModelDeltaDecoration {
return {
range: makeMarkerRange(lineNumber, line),
options: {
isWholeLine: true,
className: 'orca-conflict-marker-line',
linesDecorationsClassName: 'orca-conflict-line-decoration',
marginClassName: 'orca-conflict-margin',
hoverMessage: { value: label },
linesDecorationsTooltip: label,
after: {
content: ` ${label}`,
inlineClassName: 'orca-conflict-marker-label'
}
}
}
}
function makeSectionDecoration(
startLineNumber: number,
endLineNumber: number,
section: ConflictSection
): editor.IModelDeltaDecoration | null {
if (startLineNumber > endLineNumber) {
return null
}
return {
range: makeWholeLineRange(startLineNumber, endLineNumber),
options: {
isWholeLine: true,
className: `orca-conflict-section-line orca-conflict-${section}-line`
}
}
}
export function findGitConflictBlocks(content: string): ConflictBlock[] {
const lines = content.split(/\r?\n/)
const blocks: ConflictBlock[] = []
let current: {
startLine: number
baseLine?: number
separatorLine?: number
} | null = null
for (let index = 0; index < lines.length; index += 1) {
const line = lines[index]
const lineNumber = index + 1
if (line.startsWith('<<<<<<<')) {
current = { startLine: lineNumber }
continue
}
if (!current) {
continue
}
if (line.startsWith('|||||||')) {
current.baseLine = lineNumber
continue
}
if (line === '=======') {
current.separatorLine = lineNumber
continue
}
if (line.startsWith('>>>>>>>')) {
if (current.separatorLine) {
blocks.push({
startLine: current.startLine,
baseLine: current.baseLine,
separatorLine: current.separatorLine,
endLine: lineNumber
})
}
current = null
}
}
return blocks
}
export function hasGitConflictMarkers(content: string): boolean {
return content.split(/\r?\n/).some(isGitConflictMarkerLine)
}
export function buildGitConflictDecorations(content: string): editor.IModelDeltaDecoration[] {
const lines = content.split(/\r?\n/)
const decorations: editor.IModelDeltaDecoration[] = []
for (const block of findGitConflictBlocks(content)) {
const currentEndLine = (block.baseLine ?? block.separatorLine) - 1
const baseStartLine = block.baseLine ? block.baseLine + 1 : null
const sectionDecorations = [
makeSectionDecoration(block.startLine + 1, currentEndLine, 'current'),
baseStartLine ? makeSectionDecoration(baseStartLine, block.separatorLine - 1, 'base') : null,
makeSectionDecoration(block.separatorLine + 1, block.endLine - 1, 'incoming')
]
for (const decoration of sectionDecorations) {
if (decoration) {
decorations.push(decoration)
}
}
decorations.push(
makeMarkerDecoration(block.startLine, lines[block.startLine - 1] ?? '', 'Current change'),
...(block.baseLine
? [makeMarkerDecoration(block.baseLine, lines[block.baseLine - 1] ?? '', 'Common ancestor')]
: []),
makeMarkerDecoration(
block.separatorLine,
lines[block.separatorLine - 1] ?? '',
'Incoming change'
),
makeMarkerDecoration(block.endLine, lines[block.endLine - 1] ?? '', 'End conflict')
)
}
return decorations
}