From 6507ff8564c996afb4280439b19f217a8f0681ca Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:38:40 -0400 Subject: [PATCH] fix: prevent markdown files from appearing dirty on open (#673) --- .../src/components/editor/EditorPanel.tsx | 12 +- .../components/editor/RichMarkdownEditor.tsx | 16 ++ .../editor/markdown-dirty-state.test.ts | 182 ++++++++++++++++++ 3 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/components/editor/markdown-dirty-state.test.ts diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index 1dcb4ce5ff6..1b1f9fe64c6 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -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] diff --git a/src/renderer/src/components/editor/RichMarkdownEditor.tsx b/src/renderer/src/components/editor/RichMarkdownEditor.tsx index a67ea3ea54a..94c76cd5ca5 100644 --- a/src/renderer/src/components/editor/RichMarkdownEditor.tsx +++ b/src/renderer/src/components/editor/RichMarkdownEditor.tsx @@ -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(null) const serializeTimerRef = useRef(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(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 diff --git a/src/renderer/src/components/editor/markdown-dirty-state.test.ts b/src/renderer/src/components/editor/markdown-dirty-state.test.ts new file mode 100644 index 00000000000..69b4f3612d4 --- /dev/null +++ b/src/renderer/src/components/editor/markdown-dirty-state.test.ts @@ -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 +}