fix(files): title renames by their new path and expand collapsed lines one run at a time

Author: TheodoreSpeaks

apps/sim/components/diff/diff-view.tsx

@@ -101,7 +101,7 @@ function CollapsedRun({ count, onExpand }: CollapsedRunProps) {
 }
 
 /** Splits a hunk into visible lines and collapsed runs of unchanged lines. */
-function visibleItems(lines: DiffLine[], expanded: boolean) {
+function visibleItems(lines: DiffLine[], isExpanded: (start: number) => boolean) {
   const items: Array<{ line: DiffLine } | { collapsed: number; key: number }> = []
   for (let index = 0; index < lines.length; ) {
     if (lines[index].type !== 'context') {
@@ -115,7 +115,11 @@ function visibleItems(lines: DiffLine[], expanded: boolean) {
     const trailing = index === lines.length
     const keepBefore = leading ? 0 : CONTEXT_EDGE
     const keepAfter = trailing ? 0 : CONTEXT_EDGE
-    if (expanded || run.length <= COLLAPSE_AFTER || run.length <= keepBefore + keepAfter + 1) {
+    if (
+      isExpanded(start) ||
+      run.length <= COLLAPSE_AFTER ||
+      run.length <= keepBefore + keepAfter + 1
+    ) {
       for (const line of run) items.push({ line })
       continue
     }
@@ -147,7 +151,8 @@ function marker(line: DiffLine | undefined) {
  * when the hunk headers carry them.
  */
 export function DiffView({ hunks }: DiffViewProps) {
-  const [expanded, setExpanded] = useState<ReadonlySet<number>>(() => new Set())
+  /** Expanded runs, keyed `hunk:start` so each collapsed run opens on its own. */
+  const [expanded, setExpanded] = useState<ReadonlySet<string>>(() => new Set())
   const numbered = hunks.some((hunk) =>
     hunk.lines.some((line) => line.oldLine !== undefined || line.newLine !== undefined)
   )
@@ -157,8 +162,7 @@ export function DiffView({ hunks }: DiffViewProps) {
       <div className={cn('grid w-max min-w-full font-mono text-caption leading-[1.6]', columns)}>
         {hunks.map((hunk, hunkIndex) => {
           const segments = wordSegments(hunk.lines)
-          const items = visibleItems(hunk.lines, expanded.has(hunkIndex))
-          const expand = () => setExpanded((current) => new Set(current).add(hunkIndex))
+          const items = visibleItems(hunk.lines, (start) => expanded.has(`${hunkIndex}:${start}`))
           return (
             <Fragment key={hunkIndex}>
               {(hunk.file || hunk.heading || hunkIndex > 0) && (
@@ -181,7 +185,13 @@ export function DiffView({ hunks }: DiffViewProps) {
                     </span>
                   </div>
                 ) : (
-                  <CollapsedRun key={`c${item.key}`} count={item.collapsed} onExpand={expand} />
+                  <CollapsedRun
+                    key={`c${item.key}`}
+                    count={item.collapsed}
+                    onExpand={() =>
+                      setExpanded((current) => new Set(current).add(`${hunkIndex}:${item.key}`))
+                    }
+                  />
                 )
               )}
             </Fragment>

apps/sim/lib/diff/unified.test.ts

@@ -58,6 +58,19 @@ describe('parseUnifiedDiff', () => {
     ])
   })
 
+  it('titles a renamed file by its new path', () => {
+    expect(parseUnifiedDiff('--- a/old.ts\n+++ b/new.ts\n@@ -1 +1 @@\n-a\n+b').path).toBe('new.ts')
+  })
+
+  it('reads a changed line starting with --- after the headers as a change', () => {
+    expect(
+      parseUnifiedDiff('--- a/x.md\n+++ b/x.md\n--- divider\n+=== divider').hunks[0].lines
+    ).toEqual([
+      { type: 'del', text: '-- divider' },
+      { type: 'add', text: '=== divider' },
+    ])
+  })
+
   it('names each side of a two-document comparison', () => {
     const diff = parseUnifiedDiff(
       '--- sim:knowledge/kb/old\n+++ sim:knowledge/kb/new\n-Refunds are available within 14 days.\n+Annual plans can be refunded within 30 days.'
@@ -72,6 +85,11 @@ describe('parseUnifiedDiff', () => {
     ['a knowledge source without a document', '--- sim:knowledge/kb_1\n-x\n+y', 'sim:knowledge/'],
     ['a line without a marker', '@@ x @@\n-a\nplain', 'Line 3'],
     ['no changes', ' just context', 'at least one + or - line'],
+    [
+      'a sim: source mixed with file paths',
+      'diff --git a/x b/x\n--- sim:file/f1\n+++ sim:file/f1\n-a\n+b\ndiff --git a/y b/y\n--- a/y.ts\n+++ b/y.ts\n-c\n+d',
+      'cannot also include ordinary file paths',
+    ],
   ])('rejects %s', (_, text, message) => {
     expect(() => parseUnifiedDiff(text)).toThrow(message)
   })

apps/sim/lib/diff/unified.ts

@@ -68,27 +68,38 @@ export function parseUnifiedDiff(text: string): UnifiedDiff {
   const rows = text.replace(/\r\n?/g, '\n').replace(/\n$/, '').split('\n')
   let oldSource: DiffSource | null = null
   let newSource: DiffSource | null = null
-  let path: string | null = null
   const hunks: DiffHunk[] = []
   let hunk: DiffHunk | null = null
   let oldLine: number | undefined
   let newLine: number | undefined
 
   let file: string | null = null
+  let namesPath = false
+  /** Each file takes one `---` and one `+++` header; a later line starting that way is a change. */
+  let seenOld = false
+  let seenNew = false
   for (const [index, row] of rows.entries()) {
     if (row.startsWith('diff --git ')) {
       hunk = null
       file = null
+      seenOld = false
+      seenNew = false
       continue
     }
-    if (!hunk && (row.startsWith('--- ') || row.startsWith('+++ '))) {
+    const isOld = row.startsWith('--- ')
+    const isNew = row.startsWith('+++ ')
+    if (!hunk && ((isOld && !seenOld) || (isNew && !seenNew))) {
+      if (isOld) seenOld = true
+      else seenNew = true
       const target = row.slice(4).trim()
       const parsed = parseSource(target)
-      if (row.startsWith('--- ')) oldSource = parsed ?? oldSource
-      else newSource = parsed ?? newSource
-      if (!parsed && target !== '/dev/null' && (row.startsWith('+++ ') || !file))
-        file = target.replace(/^[ab]\//, '').split('\t')[0]
-      path ??= file
+      if (parsed) {
+        if (isOld) oldSource = parsed
+        else newSource = parsed
+      } else if (target !== '/dev/null') {
+        namesPath = true
+        if (isNew || !file) file = target.replace(/^[ab]\//, '').split('\t')[0]
+      }
       continue
     }
     if (!hunk && GIT_HEADER.test(row)) continue
@@ -121,10 +132,12 @@ export function parseUnifiedDiff(text: string): UnifiedDiff {
 
   if (!hunks.some((entry) => entry.lines.some((line) => line.type !== 'context')))
     throw new Error('A diff needs at least one + or - line')
+  if (namesPath && (oldSource || newSource))
+    throw new Error('A diff with a sim: source cannot also include ordinary file paths')
   oldSource ??= newSource
   newSource ??= oldSource
   const files = new Set(hunks.map((entry) => entry.file))
+  const path = files.size > 1 ? `${files.size} files` : (hunks[0]?.file ?? file)
   if (files.size <= 1) for (const entry of hunks) entry.file = undefined
-  else path = `${files.size} files`
   return { oldSource, newSource, path: oldSource ? null : path, hunks }
 }