mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
Merge pull request #3783 from vitorpamplona/claude/highlight-excess-spaces-0678he
Trim highlight context to bounded window, collapse whitespace
This commit is contained in:
+83
-5
@@ -24,9 +24,11 @@ package com.vitorpamplona.amethyst.commons.model.highlights
|
||||
* What to render for a NIP-84 highlight: the passage to show, and the range inside it that
|
||||
* the user actually marked.
|
||||
*
|
||||
* When the event carries a `context` tag the whole surrounding sentence is shown with the
|
||||
* quote marked inside it. When it doesn't — or the quote can't be located in the context —
|
||||
* [text] is the quote alone and [marked] is null, meaning "mark all of it".
|
||||
* When the event carries a `context` tag the surrounding passage is shown with the quote
|
||||
* marked inside it, trimmed to a bounded window on each side so a quote pulled from the
|
||||
* middle of a long article doesn't drag whole paragraphs into the feed. When it doesn't —
|
||||
* or the quote can't be located in the context — [text] is the quote alone and [marked] is
|
||||
* null, meaning "mark all of it".
|
||||
*/
|
||||
data class HighlightQuote(
|
||||
val text: String,
|
||||
@@ -49,7 +51,7 @@ data class HighlightQuote(
|
||||
|
||||
val at = locate(context, highlight, prefix)
|
||||
return if (at != null) {
|
||||
HighlightQuote(context, at until (at + highlight.length))
|
||||
window(context, at, at + highlight.length)
|
||||
} else {
|
||||
// Context that doesn't actually contain the quote is worse than no context:
|
||||
// it would mark nothing and silently show text the user never highlighted.
|
||||
@@ -57,6 +59,78 @@ data class HighlightQuote(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Most surrounding context to keep on each side of the marked quote, in characters.
|
||||
* A highlight taken from the middle of a long article can carry the whole article in
|
||||
* its `context` tag; without a cap the feed card would render several paragraphs around
|
||||
* a one-sentence highlight. Just enough to frame the quote, no more.
|
||||
*/
|
||||
private const val MAX_CONTEXT_CHARS_PER_SIDE = 160
|
||||
|
||||
private const val ELLIPSIS = "…"
|
||||
|
||||
/**
|
||||
* Trims the context down to [MAX_CONTEXT_CHARS_PER_SIDE] on each side of the quote,
|
||||
* snapping the cut to a whole-word boundary and marking it with an ellipsis. The quote
|
||||
* itself ([start] until [endExclusive]) is always kept in full, and the returned
|
||||
* [marked] range is re-based onto the trimmed text.
|
||||
*/
|
||||
private fun window(
|
||||
context: String,
|
||||
start: Int,
|
||||
endExclusive: Int,
|
||||
): HighlightQuote {
|
||||
val lead = trimLead(context.substring(0, start))
|
||||
val trail = trimTrail(context.substring(endExclusive))
|
||||
val quote = context.substring(start, endExclusive)
|
||||
|
||||
val prefix = if (lead.trimmed) "$ELLIPSIS " else ""
|
||||
val suffix = if (trail.trimmed) " $ELLIPSIS" else ""
|
||||
|
||||
// Drop any blank lines sitting at the very edges of the context — with the far text
|
||||
// trimmed (or even when it isn't) they would otherwise render as empty space above or
|
||||
// below the quote. Only the outer edges are touched; the whitespace framing the quote
|
||||
// itself is preserved.
|
||||
val raw = prefix + lead.text + quote + trail.text + suffix
|
||||
val leadingBlank = raw.length - raw.trimStart().length
|
||||
val text = raw.trim()
|
||||
|
||||
val markStart = (prefix.length + lead.text.length - leadingBlank).coerceAtLeast(0)
|
||||
val markEnd = (markStart + quote.length).coerceAtMost(text.length)
|
||||
return HighlightQuote(text, markStart until markEnd)
|
||||
}
|
||||
|
||||
private class Side(
|
||||
val text: String,
|
||||
val trimmed: Boolean,
|
||||
)
|
||||
|
||||
/** Keeps the tail of the leading context, starting at a whole word. */
|
||||
private fun trimLead(text: String): Side {
|
||||
if (text.length <= MAX_CONTEXT_CHARS_PER_SIDE) return Side(text, false)
|
||||
|
||||
var i = text.length - MAX_CONTEXT_CHARS_PER_SIDE
|
||||
// Skip the partial word the budget landed inside, then the whitespace after it, so
|
||||
// the kept text begins at the start of a whole word rather than mid-word.
|
||||
while (i < text.length && !text[i].isWhitespace()) i++
|
||||
while (i < text.length && text[i].isWhitespace()) i++
|
||||
val cut = if (i >= text.length) text.length - MAX_CONTEXT_CHARS_PER_SIDE else i
|
||||
return Side(text.substring(cut), true)
|
||||
}
|
||||
|
||||
/** Keeps the head of the trailing context, ending at a whole word. */
|
||||
private fun trimTrail(text: String): Side {
|
||||
if (text.length <= MAX_CONTEXT_CHARS_PER_SIDE) return Side(text, false)
|
||||
|
||||
var i = MAX_CONTEXT_CHARS_PER_SIDE
|
||||
// Retreat over the partial word the budget landed inside, then the whitespace before
|
||||
// it, so the kept text ends at the end of a whole word rather than mid-word.
|
||||
while (i > 0 && !text[i - 1].isWhitespace()) i--
|
||||
while (i > 0 && text[i - 1].isWhitespace()) i--
|
||||
val cut = if (i <= 0) MAX_CONTEXT_CHARS_PER_SIDE else i
|
||||
return Side(text.substring(0, cut), true)
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds [highlight] inside [context], preferring the occurrence whose preceding text
|
||||
* ends with [prefix]. Highlighters emit that prefix precisely so a repeated quote can
|
||||
@@ -67,9 +141,13 @@ data class HighlightQuote(
|
||||
highlight: String,
|
||||
prefix: String?,
|
||||
): Int? {
|
||||
// The common case — no prefix to disambiguate with — needs only the first match, so
|
||||
// don't scan the whole context enumerating every occurrence.
|
||||
if (prefix.isNullOrBlank()) return context.indexOf(highlight).takeIf { it >= 0 }
|
||||
|
||||
val occurrences = occurrencesOf(context, highlight)
|
||||
if (occurrences.isEmpty()) return null
|
||||
if (occurrences.size == 1 || prefix.isNullOrBlank()) return occurrences.first()
|
||||
if (occurrences.size == 1) return occurrences.first()
|
||||
|
||||
val tail = prefix.trimEnd()
|
||||
return occurrences.firstOrNull { context.substring(0, it).trimEnd().endsWith(tail) }
|
||||
|
||||
+46
@@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.commons.model.highlights
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
class HighlightQuoteTest {
|
||||
@Test
|
||||
@@ -89,4 +90,49 @@ class HighlightQuoteTest {
|
||||
assertEquals("some context", quote.text)
|
||||
assertNull(quote.marked)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun keepsShortContextWholeWithoutEllipsis() {
|
||||
val quote = HighlightQuote.of("the merge happens slowly", "We think the merge happens slowly. It does.")
|
||||
|
||||
assertEquals("We think the merge happens slowly. It does.", quote.text)
|
||||
assertTrue('…' !in quote.text)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun trimsLongContextToAWindowAroundTheQuote() {
|
||||
val filler = "word ".repeat(200).trim() // ~1000 chars of context on each side
|
||||
val context = "$filler the marked quote $filler"
|
||||
val quote = HighlightQuote.of("the marked quote", context)
|
||||
|
||||
// The whole quote survives and stays marked...
|
||||
assertEquals("the marked quote", quote.text.substring(quote.marked!!))
|
||||
// ...but the surrounding text is trimmed with an ellipsis on each side...
|
||||
assertTrue(quote.text.startsWith("… "))
|
||||
assertTrue(quote.text.endsWith(" …"))
|
||||
// ...and the result is a small fraction of the original two-paragraph context.
|
||||
assertTrue(quote.text.length < context.length / 2, "expected windowing, got ${quote.text.length} of ${context.length}")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun dropsBlankLinesAtTheEdgesOfAShortContext() {
|
||||
// A context whose paragraph boundaries left blank lines at its very start and end would
|
||||
// otherwise render as empty space above and below the quote.
|
||||
val quote = HighlightQuote.of("Forward Secrecy", "\n\nForward Secrecy is nice.\n\n")
|
||||
|
||||
assertEquals("Forward Secrecy is nice.", quote.text)
|
||||
assertEquals(0 until 15, quote.marked)
|
||||
assertEquals("Forward Secrecy", quote.text.substring(quote.marked!!))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun trimmingSnapsToWholeWordsSoNoWordIsCutInHalf() {
|
||||
val lead = "alpha bravo charlie delta echo foxtrot ".repeat(20) // long, space-separated
|
||||
val context = "${lead}QUOTE"
|
||||
val quote = HighlightQuote.of("QUOTE", context)
|
||||
|
||||
// The kept lead-in starts right after the ellipsis with a whole word, never a fragment.
|
||||
val keptLead = quote.text.removePrefix("… ").removeSuffix("QUOTE")
|
||||
assertTrue(keptLead.split(" ").first() in setOf("alpha", "bravo", "charlie", "delta", "echo", "foxtrot"))
|
||||
}
|
||||
}
|
||||
|
||||
+14
-1
@@ -131,6 +131,13 @@ class HighlightEvent(
|
||||
* NIP-84 `context` tag and falling back to reconstructing it from a W3C
|
||||
* `textquoteselector`'s prefix/suffix (as web highlighter clients emit) so the
|
||||
* in-context rendering still works when no `context` tag is present.
|
||||
*
|
||||
* The prefix/suffix are scraped from a web page, so they carry the page's
|
||||
* block-boundary whitespace — runs of newlines and spaces between DOM nodes — which
|
||||
* would otherwise render as a stack of blank lines around the highlight. Each run is
|
||||
* collapsed to a single space so the surrounding context reads as one continuous
|
||||
* passage; the highlight's own [content] is left verbatim so its offsets inside the
|
||||
* reconstructed context stay exact for the in-context marker.
|
||||
*/
|
||||
fun contextOrReconstructed(): String? {
|
||||
context()?.let { return it }
|
||||
@@ -138,7 +145,10 @@ class HighlightEvent(
|
||||
val selector = textQuoteSelector() ?: return null
|
||||
if (selector.prefix == null && selector.suffix == null) return null
|
||||
|
||||
return (selector.prefix ?: "") + content + (selector.suffix ?: "")
|
||||
val prefix = selector.prefix?.replace(WHITESPACE_RUN, " ")?.trimStart() ?: ""
|
||||
val suffix = selector.suffix?.replace(WHITESPACE_RUN, " ")?.trimEnd() ?: ""
|
||||
|
||||
return prefix + content + suffix
|
||||
}
|
||||
|
||||
fun inPost() = firstTaggedATag()
|
||||
@@ -150,6 +160,9 @@ class HighlightEvent(
|
||||
companion object {
|
||||
const val KIND = 9802
|
||||
|
||||
/** Any run of whitespace (spaces, tabs, newlines) — collapsed to a single space. */
|
||||
private val WHITESPACE_RUN = Regex("\\s+")
|
||||
|
||||
suspend fun create(
|
||||
msg: String,
|
||||
signer: NostrSigner,
|
||||
|
||||
+28
-1
@@ -64,12 +64,39 @@ class HighlightEventTest {
|
||||
|
||||
@Test
|
||||
fun reconstructsContextFromSelectorWhenNoContextTag() {
|
||||
// The suffix's leading "\n\n" (a page block boundary) is collapsed to a single space so
|
||||
// it doesn't render as blank lines; the quote's own content is left verbatim.
|
||||
assertEquals(
|
||||
"Your food is prechewed for you. The caged tiger prefers a pot of meat slop to an antelope they have to chase.\n\nAnd it’s not like there’s anyw",
|
||||
"Your food is prechewed for you. The caged tiger prefers a pot of meat slop to an antelope they have to chase. And it’s not like there’s anyw",
|
||||
webHighlight.contextOrReconstructed(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun collapsesRunsOfWhitespaceScrapedFromThePage() {
|
||||
// A real highlight whose prefix carries five newlines between two paragraphs of the
|
||||
// source page. Without collapsing, the reconstructed context renders a stack of blank
|
||||
// lines above the marked quote.
|
||||
val excessWhitespace =
|
||||
HighlightEvent(
|
||||
id = "fc2366a5ac54de837842492e525f8f5d141d4a9bba5b1238e135adaf4225763f",
|
||||
pubKey = "6e468422dfb74a5738702a8823b9b28168abab8655faacb6853cd0ee15deee93",
|
||||
createdAt = 1785270682,
|
||||
tags =
|
||||
arrayOf(
|
||||
arrayOf("r", "https://geohot.github.io//blog/jekyll/update/2026/06/06/our-great-war.html"),
|
||||
arrayOf("textquoteselector", "-", "n way, the better.\n\n\n\n\nHowever, ", ". A single totalizing control sy"),
|
||||
),
|
||||
content = "it will end badly for everyone if the systems of comfort prevent structural exit for the people who don’t want it",
|
||||
sig = "0428ad8aef2a12f36a4dc86105f8b429a4b4161f1fa72b08414fb2dd6c1a2276838b167682cb49ab94aa5c4e1d6351bbec0b7906a4b5e6cec1ab64a9d4c63d9d",
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
"n way, the better. However, it will end badly for everyone if the systems of comfort prevent structural exit for the people who don’t want it. A single totalizing control sy",
|
||||
excessWhitespace.contextOrReconstructed(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun prefersExplicitContextTagOverSelectorReconstruction() {
|
||||
val withContext =
|
||||
|
||||
Reference in New Issue
Block a user