Skip to content

Linkify wrapped paths anywhere in scrollback, not just the viewport - #587

Merged
dakra merged 4 commits into
mainfrom
fix/wrapped-link-scan-coverage
Aug 2, 2026
Merged

dakra merged 4 commits into
mainfrom
fix/wrapped-link-scan-coverage

Conversation

@dakra

@dakra dakra commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Fixes #582.

Paths that the terminal soft-wraps across two rows were not clickable and not
even mouse-highlighted in rails test output. Two independent causes, one
commit each.

Scan coverage

ghostel--schedule-link-detection queued only the live viewport — point-max
minus ghostel--term-rows lines. Between two queue events a flood materializes
far more rows than that, and everything above the new viewport was never
scanned by any cycle, because nothing rescans. Those rows carry no help-echo,
mouse-face or keymap, which is exactly the reported "not clickable, doesn't
even highlight" — and it also defeats the ghostel-previous-hyperlink
workaround from the issue thread, since that searches for the help-echo a
scan was supposed to have applied.

The renderer already knows which buffer range it rewrote, so it publishes that
instead. Every character of terminal output is inserted by flushSpan, which
makes coverage gapless by construction. It also closes the cursor-line skip for
free — isRowDirty repaints the row the cursor left, so a match skipped as
active input is republished and rescanned — and it is cheaper than before when
output is slow: a two-row repaint queues two rows, not the whole viewport.

Supporting changes the new region size forces:

  • The queued bounds become markers. evictScrollback deletes from the buffer
    start on almost every redraw during a sustained flood, which rots integer
    bounds queued 0.1 s earlier.
  • The scan drains at most ghostel--link-detection-chunk characters per tick,
    taken from the newest end, so one redraw materializing megabytes no longer
    blocks Emacs for as long as scanning all of it takes, and rows on screen are
    linkified before the backlog. Re-arming always goes through a timer, so
    ghostel-plain-link-detection-delay of 0 cannot recurse through it.
  • Process exit no longer cancels a partly drained scan: the program that
    flooded has exited, so nothing will ever repaint that text again.

Row limit

ghostel--wrap-joined-region counted joined rows against
ghostel--soft-wrap-row-limit cumulatively across the whole scan region
instead of restarting at each hard newline, so a region holding more wrapped
lines than the limit stopped joining at every 50th one and lost that line's
link. Harmless while scans covered ~34 rows; load-bearing once a scan covers
whatever the renderer repainted.

Testing

make -j8 all passes at both commits, so the first is not a broken
intermediate.

New elisp tests (no module): per-logical-line row limit, chunked drain reaching
the whole pending range, bounds surviving front deletion, zero delay re-arming
through a timer instead of recursing. New native tests: ghostel--repainted-region
publication, a regression test writing far more than term-rows lines in one
write, and pending-wrap completion across two redraws. Each new test was
confirmed to fail against the pre-fix code.

Verified live under elate as well — bash in a ghostel buffer, cat of a
2000-line log with 200 wrapped 121-character paths. Post-fix: 200/200 linkified
across three runs, every link carrying the full joined target, and
ghostel-previous-hyperlink reaching all 200 from point-max. Isolating the
two bugs against that harness gives 3 misses per run at identical offsets for
the row-limit bug, and 192–193 of 200 missing for the coverage gap at
ghostel-plain-link-detection-delay 0. Revealing a buffer that flooded while
hidden scans the visible rows on the first tick and walks the backlog down over
the next seven.

Known follow-ups, not addressed here

  • Line mode passes full=t on every redraw (to rebuild the prompt row after the
    input snapshot), which erases and re-renders the entire scrollback — 3.1 ms at
    a saturated default scrollback versus 0.005 ms incremental. That now queues the
    whole buffer for scanning too. It is correct rather than wasteful, since a full
    redraw wipes every detected link (measured: 40 → 0), which the old viewport-only
    scan never restored outside the visible rows — but the root fix is to stop
    forcing full redraws in line mode.
  • ghostel-compile--commit-pending-frame publishes a region nothing consumes, so
    a compile buffer's final frame is never link-scanned. Pre-existing.

Comment thread lisp/ghostel.el Outdated
Comment thread src/Renderer.zig
@dakra
dakra force-pushed the fix/wrapped-link-scan-coverage branch 2 times, most recently from a5294c3 to e023cbe Compare July 31, 2026 21:48
dakra added 2 commits August 1, 2026 17:12
Plain-text link detection queued only the live viewport — point-max
minus ghostel--term-rows lines.  Between two queue events a flood
materializes far more rows than that, and everything above the new
viewport was never scanned by any cycle, because nothing rescans.
A file path the terminal soft-wrapped across two rows then carried no
help-echo, mouse-face or keymap: not clickable, not even highlighted,
and invisible to ghostel-previous-hyperlink, which searches for the
help-echo a scan was supposed to have applied.

The renderer already knows which buffer range it rewrote, so publish
that instead of guessing.  Every character of terminal output is
inserted by flushSpan, which makes coverage gapless by construction,
and it closes the cursor-line skip for free: isRowDirty repaints the
row the cursor left, so a match skipped as active input is republished
and rescanned.  It is also cheaper when output is slow — a two-row
repaint queues two rows rather than the whole viewport.

- Renderer accumulates min/max buffer positions across flushSpan and
  publishes ghostel--repainted-region at the tail of redraw.  Both
  clear and evictScrollback run before render, so the positions are
  final; a declined or no-op redraw publishes nothing.
- ghostel--schedule-link-detection consumes and clears that variable.
  ghostel--viewport-start stays for its other callers.
- The queued bounds become markers.  evictScrollback deletes from the
  buffer start on almost every redraw during a sustained flood, which
  rots integer bounds queued 0.1 s earlier; markers collapse to the
  buffer start instead, over-covering but never under-covering.
- The scan drains at most ghostel--link-detection-chunk characters per
  tick, taken from the newest end, so a redraw that materializes
  megabytes at once no longer blocks Emacs for as long as it takes to
  scan all of it, and rows on screen are linkified before the backlog.
  Re-arming always goes through a timer, so a detection delay of 0
  cannot recurse through it.  ghostel--detect-urls returns the range
  it widened to, which the drain uses to resume past that line.
- Process exit no longer cancels a partly drained scan.  The program
  that flooded has exited, so nothing will ever repaint that text
  again; the drain stops by itself once the buffer is gone.
ghostel--wrap-joined-region counted joined rows against
ghostel--soft-wrap-row-limit cumulatively across the whole region
instead of restarting at each hard newline.  A region holding more
wrapped lines than the limit therefore stopped joining at every 50th
one, keeping the row break and losing that line's link: the path
straddled a newline the pattern cannot cross.

Harmless while scans covered about 34 rows, and load-bearing now that
a scan covers whatever the renderer repainted — a flood of compiler or
test output puts hundreds of wrapped paths in one region.  Restart the
count when the piece just consumed contains a hard newline, at one
rather than zero: the row being joined is the first of the new line.
@dakra
dakra force-pushed the fix/wrapped-link-scan-coverage branch 2 times, most recently from 21c6c3f to 7a57c76 Compare August 2, 2026 10:49
dakra added 2 commits August 2, 2026 13:02
Detection now scans the region the renderer published, and only
ghostel--redraw-now consumes it.  Four other places drive the renderer
directly, always with a full redraw, which erases the buffer and
re-inserts every row — dropping the link properties detection
attached, since the renderer re-applies OSC 8 spans but knows nothing
about detected paths and URLs.  Their region went unconsumed, so those
links never came back: changing a bold setting emptied every ghostel
buffer of file links, including the rows on screen.

- ghostel-bold-color's :set, the foreign-insert repair and line mode
  teardown queue a scan over what they rebuilt.
- ghostel-compile needs more than queueing.  Its sentinel renders,
  finalizes and switches major mode in one call, and
  kill-all-local-variables drops the queue before any tick can run, so
  a compile buffer lost every row the drain still owed — the tail of
  its output, or under a flood most of it.
  ghostel--flush-plain-link-detection drains synchronously, called
  before the rows are joined, whose ghostel-wrap properties the scan
  reads, and before the mode switch.  Its errors are demoted:
  ghostel-compile--finalized is already set by then, so a signal would
  strand the buffer with no footer and no way to retry.

A scan for a terminal that has exited also has no prompt to avoid, so
ghostel--inhibit-active-line-skip lets it cover the cursor's row —
otherwise a command printing no trailing newline left its last row,
where the cursor stopped, unlinkified for good.

ghostel-sync-theme already redraws through ghostel--redraw-now and
needs nothing.
Neither stream of the msys2 emacs reaches the Actions log, even
redirected to a file, so a failing test file surfaces only as make's
exit status with no ERT results to diagnose it by.  Have Emacs write
its own *Messages* out through elisp on `kill-emacs-hook' instead, and
echo that from a wrapper.

Add `-k' as well: the test files run serially in that job, so a
failing one otherwise hides every file after it.
@dakra
dakra force-pushed the fix/wrapped-link-scan-coverage branch from 7a57c76 to f445583 Compare August 2, 2026 11:02
@dakra
dakra merged commit f445583 into main Aug 2, 2026
29 checks passed
@dakra
dakra deleted the fix/wrapped-link-scan-coverage branch August 2, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

continuing problems with hard newline

2 participants