Description
#390 adds coverage for parts of highlight() that had none. Writing the assertions surfaced two behaviours that look emergent rather than designed. Both are pinned now, which is better than leaving them unspecified — but pinning an accident with an exact-equality assertion means a future deliberate change will read as a regression against a spec nobody meant to write. Raising them so the choice is made on purpose.
Neither is a bug report. Both are questions.
1. The truncation budget scales with the number of marks
The retained prefix before the first <mark> is not a constant. Measured against main:
| marks in the snippet |
characters kept before the first mark |
| 1 |
13 |
| 2 |
26 |
| 3 |
39 |
| 4 |
52 |
The mechanism: the budget is Array.from(content.slice(markIndex)).length — the code-point length from the first <mark> to the end, tag characters included — while the loop that spends it skips characters inside tags. So every additional complete <mark></mark> pair donates its own 13 characters to the budget, and the walk spends that slack on 13 more characters of real leading context.
The comment in test/utils/search.spec.ts said, before #390, that the surviving prefix is "always '<mark>'.length + '</mark>'.length characters — whatever the match is". Singular, fixed. Nobody who touched this file appears to have believed it scaled.
The question: should a row matching four words show four times the leading context of a row matching one? Fuse routinely returns several regions for one field on a multi-word query, so this is ordinary, not exotic. A constant budget per snippet is the other reasonable answer, and it is a one-line change — but it would fail the test #390 adds, which is exactly why it is worth deciding rather than discovering.
2. A match carrying no value renders as an empty string
Fuse types value?: string. generateHighlightedText coalesces with value = value || '', so a match with indices but no value is treated as a hit and returns ''.
highlight()'s own docstring says undefined means "nothing here applies… Callers render the plain value" — which is semantically what a value-less match is. Returning '' says the opposite: something applied, and it was empty.
No live rendering difference today. The three call sites in CommandPalette.vue:387-389 feed v-if="item.labelHtml" and siblings — truthy checks, so '' and undefined both fall through to the plain text. But that equivalence is incidental: it holds because the template happens to test truthiness rather than nullishness, and it would stop holding the moment someone writes ?? .
The question: should this return undefined, or continue to the next match, instead of ''? Either is consistent with the documented contract; the coalesce is only load-bearing against throwing, which undefined avoids equally.
Additional context
Both found by independent review passes over #388 and #390. #390 pins current behaviour deliberately — whichever way these go, that PR's assertions are the thing to change, and they are named so they are easy to find.
Description
#390 adds coverage for parts of
highlight()that had none. Writing the assertions surfaced two behaviours that look emergent rather than designed. Both are pinned now, which is better than leaving them unspecified — but pinning an accident with an exact-equality assertion means a future deliberate change will read as a regression against a spec nobody meant to write. Raising them so the choice is made on purpose.Neither is a bug report. Both are questions.
1. The truncation budget scales with the number of marks
The retained prefix before the first
<mark>is not a constant. Measured againstmain:The mechanism: the budget is
Array.from(content.slice(markIndex)).length— the code-point length from the first<mark>to the end, tag characters included — while the loop that spends it skips characters inside tags. So every additional complete<mark></mark>pair donates its own 13 characters to the budget, and the walk spends that slack on 13 more characters of real leading context.The comment in
test/utils/search.spec.tssaid, before #390, that the surviving prefix is "always'<mark>'.length + '</mark>'.lengthcharacters — whatever the match is". Singular, fixed. Nobody who touched this file appears to have believed it scaled.The question: should a row matching four words show four times the leading context of a row matching one? Fuse routinely returns several regions for one field on a multi-word query, so this is ordinary, not exotic. A constant budget per snippet is the other reasonable answer, and it is a one-line change — but it would fail the test #390 adds, which is exactly why it is worth deciding rather than discovering.
2. A match carrying no
valuerenders as an empty stringFuse types
value?: string.generateHighlightedTextcoalesces withvalue = value || '', so a match withindicesbut novalueis treated as a hit and returns''.highlight()'s own docstring saysundefinedmeans "nothing here applies… Callers render the plain value" — which is semantically what a value-less match is. Returning''says the opposite: something applied, and it was empty.No live rendering difference today. The three call sites in
CommandPalette.vue:387-389feedv-if="item.labelHtml"and siblings — truthy checks, so''andundefinedboth fall through to the plain text. But that equivalence is incidental: it holds because the template happens to test truthiness rather than nullishness, and it would stop holding the moment someone writes??.The question: should this return
undefined, orcontinueto the next match, instead of''? Either is consistent with the documented contract; the coalesce is only load-bearing against throwing, whichundefinedavoids equally.Additional context
Both found by independent review passes over #388 and #390. #390 pins current behaviour deliberately — whichever way these go, that PR's assertions are the thing to change, and they are named so they are easy to find.