Add citation/reference hover preview (RefHover)#5669
Merged
Conversation
Hovering an internal link (citation, cross-reference, figure, equation, heading, code listing, or abbreviation) shows a popup previewing the destination region instead of jumping there. - RefHover: popup lifecycle, geometry, wheel-scroll and Ctrl+wheel zoom, /XYZ zoom-hint handling, cursor-leave hiding, annotation-tooltip suppression when overlapping an internal link - RefHoverDetect: pure detectors for citations, captions, headings, figures/equations, with data-driven caption/heading dictionaries and region detection; covered by RefHover_ut unit tests - EngineBase/EngineMupdf: expose destination text/region lookup - Settings: options for the hover preview - vs2022 project files and docs updated
7cfb441 to
a49b5d9
Compare
Member
|
I'm testing changes.
I guess that 1 and 3 are caused by the fact that preview is only updated on mouse move but not on scroll or other navigation changes.
|
Member
|
I understand that picking the area to show is a heuristic. Maybe it can be tweaked for the attached document, in page 4 the following links:
If not, I understand. |
Commit a49b5d9 hand-edited the generated vs2022 project files without adding the new files to premake5.files.lua, so the next premake regeneration would have dropped them, causing link errors. The hand edits were also incomplete (RefHover.h and RefHoverDetect.h missing from vcxproj includes, RefHoverDetect.cpp missing from filters, test_util filters not updated). Add RefHover.* / RefHoverDetect.* to premake5.files.lua and regenerate the vs2022 projects with bin\premake5.exe vs2022, which fixes all the inconsistencies and confirms everything else was in sync (the regeneration is additions only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hover timer survives tab switches, document reloads and closes, so when it fired the pending destination page could be invalid for the document now in the tab, and GetZoomReal() on an out-of-range page dereferences a null PageInfo. Validate with ValidPageNo() and hide the popup instead. Also reject internal links pointing past the last page (possible in malformed documents) in IsInternalLinkDest(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The topmost popup was only hidden by mouse movement, so it stayed on screen (showing content from the wrong document, or over another application) after keyboard tab switches, closing the document, Alt+Tab, and during text selection / drags. - hide in CloseDocumentInCurrentTab(), which covers tab switch (via LoadModelIntoTab), close and reload; this also kills a pending hover timer that otherwise leaked into the home page wndproc and resets the displayed page so the popup wheel handlers can never use a stale page number against a different document engine - hide on WM_ACTIVATE / WA_INACTIVE (Alt+Tab generates no WM_MOUSELEAVE) - hide on left button down: a click either follows the hovered link or starts a selection / drag Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Meant to be used when launching SumatraPDF.exe for ad-hoc testing by humans or agents: - always starts a new instance, even with ReuseInstance = true - does not restore a session, only loads files given on the cmd-line - does not save settings, so a test run leaves no trace in the settings of the user Implemented as gForTesting global. agents.md tells agents to always use it for ad-hoc testing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DetectEntryBox constrained its initial start-glyph search to the destination column but then dropped the constraint: the walk to the line start took the leftmost same-y glyph anywhere on the page and the bracket-label search accepted any "[" at smaller x. In 2-column papers both latched onto the other column, and the bounding-box passes had no right bound, so the popup showed a full-width band of unrelated text instead of the one bibliography entry. - walk to the line start via a gap-bounded line run: never cross a horizontal gap wider than a column gutter, while still recovering the entry left edge when the link destX points mid-line - bound the bracket-label search to a hanging-indent distance - skip glyphs right of the entry column (line-run right edge plus slack) in the box passes and the forward scan Add 2-column unit tests for entries in either column; they fail against the previous detector and pass now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RefHoverSchedule() skips a re-render when hovering the same link by comparing the incoming raw link destination against displayed.destX/Y, but RefHoverOnTimer() stored the resolved values there (destY recovered from source text, destX coerced to 0). For page-level destinations (abbreviation / glossary links, /XYZ at page top) they never compared equal, so every pause of the mouse within the same link re-ran detection plus a synchronous render and re-anchored the popup at the new cursor position. Store the raw link values instead; they are only read by the dedupe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The annotation notification was suppressed over internal links even with EnableCitationHover = false, when no hover popup would ever appear in its place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MatchWordAt() folded case with towlower(), which in the "C" locale
the process runs under only folds ASCII, so accented dictionary words
("sección", "capítulo") never matched all-caps Spanish headings like
"SECCIÓN 2" - despite the comment claiming all-caps text matches.
Fold with CharLowerW() instead, which uses the OS linguistic tables
regardless of the CRT locale.
Add a unit test with an all-caps accented heading; it fails with the
towlower() version and passes now.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The popup border, cursor gap, wheel-scroll step and popup size caps were raw pixels, so on high-dpi monitors the border and cursor gap were visually smaller and the popup size caps too tight. Scale them at point of use with the popup window dpi (handles per-monitor dpi). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ShowPopup could compute a negative width / height when the page is barely visible or there is no room on either side of the cursor, showing a degenerate 0-sized topmost window; hide the popup instead. Track the actual RegisterClassW() result so that a (theoretical) registration failure doesn't re-run window creation on every mouse move. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PageDestinationMupdf documents destX / destY == -1 as "not resolved"
but NewPageDestinationMupdf assigned them from zero-initialized
locals even when ResolveLink() failed, so GetDestPoint2() reported a
{0,0} anchor for unresolved destinations. Assign only on success.
Harmless today since callers filter on pageNo first, but a trap for
future callers trusting the documented contract.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add Italian (tabella, sezione, capitolo) and Portuguese (tabela, seção, secção) words the dictionary comments claimed but didn''t have, and make the per-language comments honest about shared roots - require a trailing word boundary in heading-prefix matching so a bibliography entry starting "Sections of ..." isn''t misrouted to the landscape heading view by matching "section" - pick the topmost caption below the region in the caption-extension search instead of the first one in glyph-array order; PDFs draw text in arbitrary order (as documented elsewhere in this file), so a far-down caption drawn early could extend the popup far past the relevant one Add unit tests for all three; they fail against the previous detector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add tests for the author-year hanging-indent path (rule b), the single-line description-list path (rule d) and the vertical-gap path (rule c). Rework BodyTextParenRejected so the "(N)" label is alone on its line: previously the line-trailing rule rejected the input before the left-half rule it claims to test was ever consulted, so the test passed even with that rule deleted. Verified by mutation: disabling each pinned rule fails the matching test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Convert kCaptionWords / kHeadingPrefixWords from nullptr-terminated WCHAR* arrays to \0-separated utf8 strings navigated with seqstrings::Next(), converting entries with ToWStrTemp() at the match sites. Covered by the existing dictionary unit tests, including the accented-word round-trip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CitationHoverDelay is how long (in ms) to hover an internal-document link before the popup shows, replacing the hard-coded 300 ms. A negative value disables the popup (the equivalent of the old EnableCitationHover = false). Defaults to 300. Also fix clang-format / clang-tidy detection in cmd/util.ts: VS 18 ships the llvm tools in Llvm\x64\bin, not Llvm\bin, which made gen-settings.ts die halfway through generation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Toggles the citation / reference hover preview via the CitationHoverDelay advanced setting: sets it to -1 (disabled) when enabled, back to the default 300 ms when disabled. The command palette shows the resulting state like other toggle commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
Thanks, I've made some tweaks / fixes. Notably, I changed Also added I'm not sure about wheel scroll to be used for preview. What do other programs implementing this do? This is inherently an heuristic so I encourage more testing and tweaking the preview area detector. I'm sure once it's in pre-release, people will send feedback |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #5611 with a single commit.
This update is entirely written by claude - "only" driven by me in a code-and-fix manager
Screenshot: