Skip to content
Snippets Groups Projects
Commit 8cbc9e12 authored by Simon Tatham's avatar Simon Tatham
Browse files

Make FilePosition store a fraction, not a line number.

This fixes two bugs at once: instability under repeated window
resizing, and a panic due to an out-of-bounds array access attempt.

The panic: if a post on my home timeline is edited to be slightly
shorter, and the file position was right at the bottom of its original
size, then Mastodonochrome panics in File::draw(), with a backtrace
suggesting that a bounds check failed because of the line number in
FilePosition being out of range.

I'm sure I could fix that by itself, by inserting a missing call to
clip_pos_within_item() somewhere or other. But I had a better idea,
which fixes another bug at the same time.

The resize instability: the old FilePosition had the semantics "As
long as the post we're referring to was last rendered at width W,
we're at line L within it." If the width changes, we throw up our
hands and say "in that case I don't know where we were within the
post, better reset to the bottom". So if you resize the window back
and forth, your previous file position isn't restored. This is only
rarely inconvenient for me, but some other desktop environments will
be much harder hit: in particular, tiling window managers often resize
windows a great deal without much deliberate user action.

To fix both problems at once, I've reworked FilePosition so that
instead of a width and a line number, it stores a fraction, as a usize
numerator and denominator, with the semantics "We're currently N/D of
the way through this item". This is easy to interpret for the current
line count of the item, and it doesn't need to be mutated on a window
resize at all. Every time you use a movement key, the fraction is
reset to be one based on the current line count of the item, but as
long as you don't logically move within the file, the previous line
count is retained.

So we should have the required stability property: if you resize the
window and then resize it back again, your file position will be
exactly where it was before.

And now clip_pos_within_item() doesn't even exist, so there's no need
to worry about whether _any_ piece of code should be calling it or
not. File positions now automatically avoid going beyond the bounds of
the containing item, because they always represent a rational in the
interval [0,1].
parent 479d2b36
No related branches found
No related tags found
Loading
Pipeline #737830 passed with stages
in 10 minutes and 4 seconds
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment