Skip to content

Commit 080098d

Browse files
committed
fix: avoid "No textual changes" at random
1 parent 0348bc6 commit 080098d

3 files changed

Lines changed: 39 additions & 19 deletions

File tree

cli/diff-engine.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ test('a big file that merely shifted keeps its real patch', async () => {
5757
assert.match(files[0].patch!, /\+tail/)
5858
})
5959

60+
test('bytes that are not valid UTF-8 count as binary, not as an empty diff', async () => {
61+
// Differs only in the last byte, and both sequences are invalid UTF-8 with no
62+
// NUL in them: the lenient decoder maps each to the same U+FFFD run, so the
63+
// diff would come back empty and the file would read as unchanged.
64+
const bytes = (last: number) => new Uint8Array([0xC3, 0x28, 0xA0, last, 0x0A])
65+
const { files } = await buildDiff(
66+
{ ref: { name: 'p', version: '1' }, entries: [{ name: 'package/latin.txt', bytes: bytes(0xA1) }] },
67+
{ ref: { name: 'p', version: '2' }, entries: [{ name: 'package/latin.txt', bytes: bytes(0xA2) }] },
68+
[], new AbortController(),
69+
)
70+
assert.equal(files.length, 1)
71+
assert.equal(files[0].binary, true)
72+
assert.equal(files[0].patch, undefined)
73+
})
74+
6075
test('byte-identical and excluded files are dropped by the scan, not the diff pass', async () => {
6176
const result = await buildDiff(a, b, ['*.map'], new AbortController())
6277
assert.deepEqual(result.files.map(f => `${f.status} ${f.path}`), [

src/components/PierreFileDiff.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@
147147
</p>
148148

149149
<p v-else-if="!file.patch" class="px-4 py-6 text-sm text-on-surface-variant italic">
150-
No textual changes.
150+
{{ file.added || file.removed ? 'No patch available for this file.' : 'No textual changes.' }}
151151
</p>
152152

153153
<!-- Pierre renders the <diffs-container> custom element inside this host. -->

src/lib/diff-engine.ts

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,6 @@ function bytesEqual (a: Uint8Array, b: Uint8Array): boolean {
4444
return a.length === b.length && a.every((v, i) => v === b[i])
4545
}
4646

47-
function isBinary (bytes: Uint8Array): boolean {
48-
return bytes.subarray(0, 8000).includes(0)
49-
}
50-
5147
function countLines (text: string): number {
5248
if (text === '') {
5349
return 0
@@ -113,7 +109,18 @@ async function toMap (entries: TarEntry[]): Promise<Map<string, Uint8Array>> {
113109
return map
114110
}
115111

116-
const decoder = new TextDecoder()
112+
const decoder = new TextDecoder('utf-8', { fatal: true })
113+
114+
function decodeText (bytes: Uint8Array): string | null {
115+
if (bytes.subarray(0, 8000).includes(0)) {
116+
return null
117+
}
118+
try {
119+
return decoder.decode(bytes)
120+
} catch {
121+
return null
122+
}
123+
}
117124

118125
interface ChangedFile {
119126
path: string
@@ -160,13 +167,13 @@ async function modifiedEntry (
160167
): Promise<FileEntry> {
161168
const base = { path, scope: scopeOf(path), status: 'modified' as const }
162169

163-
if (isBinary(av) || isBinary(bv)) {
170+
const textA = decodeText(av)
171+
const textB = decodeText(bv)
172+
if (textA === null || textB === null) {
164173
return { ...base, added: 0, removed: 0, linesA: 0, linesB: 0, chars: 0, binary: true }
165174
}
166175
await checkAborted(abortController)
167176

168-
const textA = decoder.decode(av)
169-
const textB = decoder.decode(bv)
170177
const linesA = countLines(textA)
171178
const linesB = countLines(textB)
172179

@@ -236,11 +243,10 @@ export async function buildDiff (
236243
files.push(entry)
237244
await checkAborted(abortController)
238245
} else if (bv && !av) {
239-
const binary = isBinary(bv)
240-
const text = binary ? '' : decoder.decode(bv)
241-
const added = countLines(text)
246+
const text = decodeText(bv)
247+
const added = text === null ? 0 : countLines(text)
242248
linesAdded += added
243-
const patch = binary ? undefined : await diffText('', text, abortController)
249+
const patch = text === null ? undefined : await diffText('', text, abortController)
244250
await checkAborted(abortController)
245251

246252
const patchChars = patch ? patch.length : 0
@@ -254,16 +260,15 @@ export async function buildDiff (
254260
linesA: 0,
255261
linesB: added,
256262
chars: patchChars,
257-
binary,
263+
binary: text === null,
258264
patch: patch && patch.length > maxPatch ? patch.slice(0, maxPatch) : patch,
259265
truncated: !!patch && patch.length > maxPatch,
260266
})
261267
} else if (av && !bv) {
262-
const binary = isBinary(av)
263-
const text = binary ? '' : decoder.decode(av)
264-
const removed = countLines(text)
268+
const text = decodeText(av)
269+
const removed = text === null ? 0 : countLines(text)
265270
linesRemoved += removed
266-
const patch = binary ? undefined : await diffText(text, '', abortController)
271+
const patch = text === null ? undefined : await diffText(text, '', abortController)
267272
await checkAborted(abortController)
268273

269274
const patchChars = patch ? patch.length : 0
@@ -277,7 +282,7 @@ export async function buildDiff (
277282
linesA: removed,
278283
linesB: 0,
279284
chars: patchChars,
280-
binary,
285+
binary: text === null,
281286
patch: patch && patch.length > maxPatch ? patch.slice(0, maxPatch) : patch,
282287
truncated: !!patch && patch.length > maxPatch,
283288
})

0 commit comments

Comments
 (0)