Basic idea: If the goal is for the cursor to be on the viewport, focus the code on ensuring that constraint by construction.
Motivation: The downstream driver.love fork still has persistent bugs. And I'm seeing some inconclusive signs that edit.lua might be failing to change screen_top some of the time when it needs to. But this only happens in driver.love, never in lines.love. So the null hypothesis is that there's some subtle assumption in lines.love that we're violating when rendering it on a surface.
What do you do with such subtleties? It might actually be counterproductive to fix them at source. You end up with complexity upstream that won't actually matter there if it breaks. Which is a recipe for it to break silently and far away from the downstream fork that might actually care about it. Or it might confuse people in future who don't care about the downstream forks, just lines.love.
Maybe it makes sense to modify edit.lua here and take the hit on all possible future merge conflicts. But considering the cost of tracking this down, it seems simplest to: a) come up with the constraint I care about, and b) modify outside edit.lua, either what it sees or its results, to preserve the new constraint.
Long ago I used to have this assertion in pensieve.love that the cursor must be within the viewport, but I ended up taking it out because it kept breaking for me when I was trying to do real work. It seems clear that there are possible assertions that are useful and yet counterproductive. If you can't keep it out of the product in the course of testing, then it annoys users where ignoring it would be a more graceful experience. Even when the user is just yourself! So it turns out this is not a problem only for large teams of extrinsically motivated people who don't eat their own dog food. No, for some things you have to fix the problem by construction, not just verify it with an assertion.
This plan isn't fully working yet in this commit. I've only fixed cases for down-arrow. I need to address up arrow, and there might also be changes for left/right arrows. Hmm, I'm going to try to follow the implementation of bring_cursor_of_cursor_node_in_view() in pensieve.love.
In the process of doing this I also noticed a bug with page-up/down. It already existed, but this approach has made it more obvious.
FWCILHZLHSMW2RQF4ZTJVABMHTD6IR4KPCHUEJFTQT2MXHV75XDQC
MU5W6A35USV3OPXIVGGY5FXC55KCIRQXOFHAGXMTOKCIL4MSQZ4AC
7JZFRO5FU57TYQOBZXVLISILKAYDQYTTGZXCPGOZNW3GAU7INQBQC
R5QXEHUIZLELJGGCZAE7ATNS3CLRJ7JFRENMGH4XXH24C5WABZDQC
QJAYOFWY7V4BWVLJMEDCCBC2HX4BDAZI6PQVLWCLYRHWGMOBFTVAC
TBB7GHINPHDTKI3B6U3M2OF7UGQ5V5DUHODB7UNKNZMBA4NWKUFAC
TZNPEHB3BRUHATVYAODURZUR45TYV62HYWQK5SUXHAOLVGYJVP7AC
EXZESF4IFJIIMKDIWKXZ3FQM5GYCIGQJEFDENCLCO5FHE5ZPABNQC
54Y4QQG3ETC4YMUDDJBQO4SLFMX3NOLI5PRGVHUY2DVGZAUJBZVAC
AUZQ7ULP65IGCJYIT3U6R42HEJT35R33TQHA7XNK242KWCV6IKMAC
P36KIMZCWGBG6CZQE3W5SVTCJMYZOVEZ3U3FAF46SO5ZMMM42PHQC
PPVABNGQTQMPYILHDNSNXBI5IOCBSIYO45NIHUR4LYPABQZE6PAQC
WKMW7PCE75A5CFXF5GPL5HQ4ZNASFOFQFHASOQ7LABSQKWU3KEGAC
HTAB35N62YXTFGITVQAK6DNEKKAUKJQQ27RYO7BBVQKIHXBH6NMQC
YOCTF4J3GCMKKVKGO2N5NGHNCRG6FLD54QPI5RGLK4OP667D3NVAC
BF7TW3EKRIDYC6J2Q2J4YOBAVQF55Y3H6KGZIHNXMH4N72MR6GXQC
2N4JZKVTSJJJIXM2VVCU5INC4IGY7J6FF4JD3TAFK5THD2QWSQPQC
O6D22PJBXFE5XGW27UD7NDIMQYL4NXH4C4T2E73F2NXUSEB4W7QQC
LIWKX2BUGJ2S3JBILVBQ23FBBGKGOIWPILVZSFESSDSHAPYA5POQC
TBPJ5WSRM5IKQH7FTAVYTWWUBWW7G2CLT5M6FN7CMPLD6Y77YIWQC
ZQMQVMFWZIGWBRCB455IKHPTAOUYPLYCSC44NBPPET557RGVRX3AC
function pan_viewport_to_contain_cursor(node)
print('viewport', Viewport.y)
local safety_margin = 2*node.editor.line_height
print('safety margin', screen_top_y, 'dvpx')
print('node', node.y)
local cursor_y = y_of_schema1(node.editor, node.editor.cursor1)
print('cursor', cursor_y, 'dvpx')
if Viewport.y < node.y then
-- node starts within Viewport
print('node starts within viewport')
local max_cursor_y = App.screen.height - scale(node.y - Viewport.y) - safety_margin
if cursor_y > max_cursor_y then
-- set Viewport.y so cursor_y == max_cursor_y
-- equation: cursor_y == App.screen.height - (node.y - Viewport.y)*Viewport.zoom - safety_margin
-- solve for Viewport.y
Viewport.y = node.y - (App.screen.height - cursor_y - safety_margin)/Viewport.zoom
print('adjusting Viewport to', Viewport.y)
end
else
-- node extends above Viewport
print('node extends above viewport')
local screen_top_y = y_of_schema1(node.editor, node.editor.screen_top1)
print('screen top', screen_top_y, 'dvpx')
local min_screen_top_y = cursor_y + safety_margin - App.screen.height
if screen_top_y < min_screen_top_y then
screen_top_y = min_screen_top_y
Viewport.y = node.y + screen_top_y/Viewport.zoom
print('adjusting Viewport to', Viewport.y)
end
end
end
if node ~= skip_updating_screen_top_for then
--? print('modifying screen_top to 1')
node.editor.screen_top1.line = 1
node.editor.screen_top1.pos = 1
end
node.editor.screen_top1.line = 1
node.editor.screen_top1.pos = 1
--? print('<=', node, skip_updating_screen_top_for)
if node ~= skip_updating_screen_top_for then
node.editor.screen_top1, node.editor.top = schema1_of_y(node.editor, scale(Viewport.y-node.y))
print('modified screen_top to', node.editor.screen_top1.line, 'at', node.editor.top, 'vpx')
else
-- adjust editor to start rendering near top of viewport
--? print(Viewport.y)
node.editor.top = Viewport.y%node.editor.line_height
if node.editor.top > 0 then node.editor.top = node.editor.top - node.editor.line_height end
--? print('top', node.editor.top)
end
node.editor.screen_top1, node.editor.top = schema1_of_y(node.editor, scale(Viewport.y-node.y))
compute_layout(Page, Page.x,Page.y, Surface, skip_updating_screen_top_for)
compute_layout(Page2, Page2.x,Page2.y, Surface, skip_updating_screen_top_for)
compute_layout(Page, Page.x,Page.y, Surface)
compute_layout(Page2, Page2.x,Page2.y, Surface)
--? print('edit', old_top.line, '=>', Cursor_node.editor.screen_top1.line)
if not eq(Cursor_node.editor.screen_top1, old_top) then
--? print('modifying Viewport', Viewport.y, 'based on', Cursor_node.y, Cursor_node.editor.screen_top1.line, Cursor_node.editor.line_height)
Viewport.y = Cursor_node.y + y_of_schema1(Cursor_node.editor, Cursor_node.editor.screen_top1)/Viewport.zoom
--? print('modified Viewport', Viewport.y)
-- Most of the time, we update the screen_top of nodes from Viewport.y.
-- But here we went the other way.
-- It's very important to avoid creating a recurrence, to avoid running
-- both sides of this feedback loop in a single frame. These apps are
-- not very numerically precise (e.g. we force text lines to start at
-- integer pixels regardless of zoom, because that keeps text crisp),
-- and the computations of Viewport.y and node.top will almost certainly
-- not converge. The resulting bugs are extremely difficult to chase
-- down.
-- The optional skip_updating_screen_top_for arg ensures we don't run
-- the other side of the feedback loop.
A(--[[skip updating screen_top for]] Cursor_node)
return
end
print('cursor after', Cursor_node.editor.cursor1.line, Cursor_node.editor.cursor1.pos)
pan_viewport_to_contain_cursor(Cursor_node)