fix(viewer): clip slide viewport to current slide boundaries#34
Merged
Conversation
The slide mode rendering loop computes slide_start just fine, but then *never* computes slide_end. So wrapped.get(line_idx) happily returns content from the next slide whenever the current slide has fewer lines than the viewport height. The status bar correctly shows "Slide 1/3" while you're staring at content from slides 1, 2, and 3 all at once. Not great. It turns out there was already a slide_end computation sitting right there in render_frame — stored in _display_lines and _display_offset — except those variables were never actually *used*. Presumably someone started the fix and forgot to finish it. Hoist slide_end before the row loop and gate every wrapped.get() call with .filter(|_| line_idx < slide_end). Lines past the boundary now return None, which falls through to the existing blank-row path. Same guard applied to the image rendering passes so images don't bleed across slides either. Closes #30
The previous commit introduced slide_end to stop content from bleeding across slide boundaries, but it left slide_start computed *four separate times* inside render_frame — once per row, every single frame. That's not a bug, but it's the kind of copy-paste that breeds bugs. Hoist both slide_start and slide_end into a single (start, end) pair at the top of render_frame. In non-slide mode slide_start is just state.offset, so the arithmetic stays identical. The four inline boundary lookups collapse into `slide_start + row`. While at it, link_at_position() was *not* guarded by slide_end at all. Click on a visually blank row past the current slide's content and it would happily resolve a link from the next slide. Guard it the same way: compute slide_end locally and bail early if the index is out of bounds. Net result: fewer lines, one less place to get wrong, and clicking on blank space no longer activates ghost links.
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
1 participant
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.
Summary
slide_endfromslide_boundaries[current_slide + 1]and gates allwrapped.get()calls with.filter(|_| line_idx < slide_end)so lines past the boundary fall through to the existing blank-row path_display_lines/_display_offsetvariables that had a partial, never-wired-up version of this fixTest plan
---separators (3+ slides with varying content lengths)mdterm --slides file.mdand verify each slide only shows its own contentcargo testpasses (120/120)cargo clippyandcargo fmt --checkcleanCloses #30