fix: prevent markdown files from appearing dirty on open (#673)

This commit is contained in:
Jinwoo Hong 2026-04-15 14:38:40 -04:00 committed by GitHub
parent 13f6c4a57b
commit 6507ff8564
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 207 additions and 3 deletions

View file

@ -283,15 +283,21 @@ function EditorPanelInner({
return
}
setEditorDraft(activeFile.id, content)
// Why: TipTap's getMarkdown() always appends a trailing newline to the
// serialized output. If the file on disk lacks that newline the naive
// strict-equality check treats the file as dirty even though no user edit
// occurred. Normalising trailing whitespace for markdown files mirrors the
// same trimEnd() used in the round-trip checker (markdown-round-trip.ts).
const isMarkdown = activeFile.language === 'markdown'
const normalize = isMarkdown ? (s: string): string => s.trimEnd() : (s: string): string => s
if (activeFile.mode === 'edit') {
// Compare against saved content to determine dirty state
const saved = fileContents[activeFile.id]?.content ?? ''
markFileDirty(activeFile.id, content !== saved)
markFileDirty(activeFile.id, normalize(content) !== normalize(saved))
} else {
// Diff mode: compare against the original modified content from git
const dc = diffContents[activeFile.id]
const original = dc?.kind === 'text' ? dc.modifiedContent : ''
markFileDirty(activeFile.id, content !== original)
markFileDirty(activeFile.id, normalize(content) !== normalize(original))
}
},
[activeFile, diffContents, fileContents, markFileDirty, setEditorDraft]

View file

@ -68,6 +68,10 @@ export default function RichMarkdownEditor({
// stuck at the first-render null value unless we read the live instance here.
const editorRef = useRef<Editor | null>(null)
const serializeTimerRef = useRef<number | null>(null)
// Why: normalizeSoftBreaks dispatches a ProseMirror transaction inside onCreate
// which triggers onUpdate. Without this guard the editor immediately marks the
// file dirty before the user has typed anything.
const isInitializingRef = useRef(true)
const [linkBubble, setLinkBubble] = useState<LinkBubbleState | null>(null)
const [isEditingLink, setIsEditingLink] = useState(false)
const isEditingLinkRef = useRef(false)
@ -263,10 +267,22 @@ export default function RichMarkdownEditor({
// other block-level operations) treat each line as its own block.
normalizeSoftBreaks(nextEditor)
lastCommittedMarkdownRef.current = nextEditor.getMarkdown()
// Why: clear the flag *after* normalizeSoftBreaks so any onUpdate
// triggered by the normalization transaction is still suppressed.
isInitializingRef.current = false
},
onUpdate: ({ editor: nextEditor }) => {
syncSlashMenu(nextEditor, rootRef.current, setSlashMenu)
// Why: normalizeSoftBreaks in onCreate dispatches a transaction that
// triggers this callback before the editor is ready for user interaction.
// Treating that structural housekeeping step as a user edit would
// immediately mark the file dirty and prompt a spurious save dialog on
// close. Bail out until initialization is complete.
if (isInitializingRef.current) {
return
}
// Why: full markdown serialization is debounced for typing performance,
// but close-confirmation and beforeunload checks still need to know
// immediately that the document changed. Optimistically mark the tab

View file

@ -0,0 +1,182 @@
import { describe, expect, it } from 'vitest'
import { Editor } from '@tiptap/core'
import StarterKit from '@tiptap/starter-kit'
import TaskList from '@tiptap/extension-task-list'
import TaskItem from '@tiptap/extension-task-item'
import { Table } from '@tiptap/extension-table'
import { TableCell } from '@tiptap/extension-table-cell'
import { TableHeader } from '@tiptap/extension-table-header'
import { TableRow } from '@tiptap/extension-table-row'
import { Markdown } from '@tiptap/markdown'
import { normalizeSoftBreaks } from './rich-markdown-normalize'
const testExtensions = [
StarterKit,
TaskList,
TaskItem.configure({ nested: true }),
Table.configure({ resizable: false }),
TableRow,
TableHeader,
TableCell,
Markdown.configure({ markedOptions: { gfm: true } })
]
function createEditor(markdown: string): Editor {
return new Editor({
element: null,
extensions: testExtensions,
content: markdown,
contentType: 'markdown'
})
}
function trimEnd(s: string): string {
return s.trimEnd()
}
/**
* Simulates the onCreate flow: normalizeSoftBreaks then getMarkdown().
*/
function simulateOnCreate(diskContent: string): string {
const editor = createEditor(diskContent)
try {
normalizeSoftBreaks(editor)
return editor.getMarkdown()
} finally {
editor.destroy()
}
}
// -----------------------------------------------------------------------
// 1. trimEnd normalization prevents phantom dirty from trailing newlines
//
// getMarkdown() always appends a trailing \n. For content that round-trips
// cleanly (no soft-break normalization), the ONLY difference is that
// trailing newline. trimEnd() must eliminate that false positive.
// -----------------------------------------------------------------------
describe('trailing newline does not cause false dirty state', () => {
const roundTripCases: [string, string][] = [
['simple paragraph', 'Hello world'],
['paragraph with trailing newline', 'Hello world\n'],
['heading and paragraph', '# Title\n\nSome body text'],
['multiple paragraphs', 'First paragraph\n\nSecond paragraph\n'],
['bold and italic', '**bold** and *italic* text'],
['inline code', 'Use `console.log()` here'],
['fenced code block', '```js\nconst x = 1\n```\n'],
['unordered list', '- item one\n- item two\n- item three'],
['ordered list', '1. first\n2. second\n3. third'],
['nested list', '- parent\n - child\n - child 2'],
['link', 'Click [here](https://example.com) please'],
['heading hierarchy', '# H1\n\n## H2\n\n### H3\n\nBody'],
['task list', '- [x] done\n- [ ] todo'],
['horizontal rule', 'Above\n\n---\n\nBelow'],
['empty document', '']
]
it.each(roundTripCases)('%s: trimEnd comparison reports clean', (_label, diskContent) => {
const serialized = simulateOnCreate(diskContent)
expect(trimEnd(serialized)).toBe(trimEnd(diskContent))
})
})
// -----------------------------------------------------------------------
// 2. normalizeSoftBreaks produces structural differences that getMarkdown()
// serializes differently. These cannot be hidden by trimEnd — they are
// handled at runtime by the isInitializingRef guard in onUpdate.
//
// The tests below document the known divergence so that future changes
// to the serializer or normalizer don't silently shift which category
// a given input falls into.
// -----------------------------------------------------------------------
describe('normalizeSoftBreaks: known structural changes', () => {
it('splits consecutive lines into separate paragraphs', () => {
const editor = createEditor('Line one\nLine two\nLine three')
try {
const before = countParagraphs(editor)
normalizeSoftBreaks(editor)
const after = countParagraphs(editor)
expect(after).toBeGreaterThan(before)
expect(after).toBe(3)
} finally {
editor.destroy()
}
})
it('serialized soft-break content differs from disk content', () => {
const disk = 'Line one\nLine two'
const serialized = simulateOnCreate(disk)
// After normalization each line is its own paragraph, serialized with
// blank-line separators. This difference is NOT a bug — the
// isInitializingRef guard prevents it from marking the file dirty.
expect(trimEnd(serialized)).not.toBe(trimEnd(disk))
expect(trimEnd(serialized)).toBe('Line one\n\nLine two')
})
it('does not modify content without soft breaks', () => {
const editor = createEditor('# Title\n\nBody text')
try {
const docBefore = editor.state.doc.toJSON()
normalizeSoftBreaks(editor)
const docAfter = editor.state.doc.toJSON()
expect(docAfter).toEqual(docBefore)
} finally {
editor.destroy()
}
})
})
// -----------------------------------------------------------------------
// 3. Actual user edits must still be detected as dirty.
// -----------------------------------------------------------------------
describe('real edits are detected as dirty', () => {
it('ProseMirror transaction produces a dirty diff', () => {
const diskContent = '# README\n\nOriginal text'
const editor = createEditor(diskContent)
try {
normalizeSoftBreaks(editor)
// Insert text via a ProseMirror transaction (no DOM required)
const { tr } = editor.state
const insertPos = editor.state.doc.content.size - 1
editor.view.dispatch(tr.insertText(' added', insertPos))
const editedMarkdown = editor.getMarkdown()
expect(trimEnd(editedMarkdown)).not.toBe(trimEnd(diskContent))
} finally {
editor.destroy()
}
})
it('deleting content produces a dirty diff', () => {
const diskContent = '# Title\n\nParagraph to keep\n\nParagraph to delete'
const editor = createEditor(diskContent)
try {
normalizeSoftBreaks(editor)
// Delete the last paragraph node
const doc = editor.state.doc
const lastChild = doc.lastChild!
const from = doc.content.size - lastChild.nodeSize
const to = doc.content.size
editor.view.dispatch(editor.state.tr.delete(from, to))
const editedMarkdown = editor.getMarkdown()
expect(trimEnd(editedMarkdown)).not.toBe(trimEnd(diskContent))
} finally {
editor.destroy()
}
})
})
function countParagraphs(editor: Editor): number {
let count = 0
editor.state.doc.forEach((node) => {
if (node.type.name === 'paragraph') {
count++
}
})
return count
}