Matt Wynne pointed out that snap.love would crash when a node went off
screen. While debugging it I noticed that selection1 was being set when
it shouldn't be.
Turns out I introduced a bug when I fixed the inscript bug back in June
(commit 9656e13774). One invariant I want to preserve is: selection1
should be unset after a mouse click (press and release without
intervening drag). This invariant was violated in my bugfix back in
June. I was concerned only with selection back then, and I didn't
realize I was breaking the mouse click case (in a fairly subtle way; you
can have selection set, and when it's set identically to the cursor
everything looks the same).
I think there might still be an issue in snap.love after this fix. I
noticed screen_bottom1.pos was nil, and as far as I recall that should
never happen.
All the Text functions assume the cursor is always on a text line. I was
violating that invariant.
* When scrolling up, I start the cursor at the top-most line below the
screen top.
* When scrolling down, I start the cursor at the top-most line below the
screen bottom.
I think it would feel slightly more natural for it to be the
bottom-most line above the screen bottom.
However, the Text functions maintain an invariant that the bottom-most
line in a buffer will be text. There's no such invariant for the
top-most line.
To fix this I have to first stop incrementally updating screen_bottom1
in the middle of a frame. Now it always has a good value from the end of
a frame.
I'm also running into some limitations in the test I'd ideally like to
write (that are documented in a comment), but I still get some sort of
automated test for this bugfix.
It's a hack:
- if you start selecting from below final line the start of the
selection is the most recent click even if it was forever ago
- (the crash we're currently fixing) if you start up and immediately
select all then click below final line => crash. recent_mouse was
never set.
- getting rid of it breaks no tests (except the crash we're currently
fixing)
Text.mouse_pos can sometimes set recent_mouse.time but not
recent_mouse.x/y. I'd assumed x/y is never nil in those situations, but
that's violated. It's most easily seen when typing C-a and then
clicking.
The bug has been spotted twice:
1. In snap.love, I selected text in one node, then another, and hit:
Error: text.lua:789: attempt to compare nil with number
stack traceback:
text.lua:789: in function 'lt1'
select.lua:19: in function 'clip_selection'
text.lua:32: in function 'draw'
edit.lua:117: in function 'draw'
[string "REPL"]:21: in function 'draw'
main.lua:152: in function 'draw'
app.lua:102: in function <app.lua:84>
[C]: in function 'xpcall'
app.lua:112: in function <app.lua:111>
[C]: in function 'xpcall'
Couldn't reproduce.
2. In text.love, inscript selected all text in a small buffer and then
clicked outside the text. And got:
Error: text.lua:784: attempt to compare nil with number
Traceback
[love "callbacks.lua"]:228: in function 'handler'
text.lua:784: in function 'lt1'
select.lua:19: in function 'clip_selection'
text.lua:27: in function 'draw'
edit.lua:117: in function 'draw'
run.lua:136: in function 'draw'
main.lua:148: in function 'draw'
app.lua:42: in function <app.lua:22>
[C]: in function 'xpcall'
This is reproducible, and also across forks.
Scenario:
* start out with some text on screen
* select some text A, delete
* select some more text B, delete
* press C-z twice to restore A and B
* press C-y twice
Before this commit only the first C-y was having an effect (deleting B).
The second was failing to delete A.
I see a path to at least maintain a single fragment per screen line. But
can we do better? It even seems unnecessary to maintain two copies of
the data, chopped up into lines and screen lines.
scenario: open a file starting with a drawing
After this commit the program doesn't crash.
Error: [string "edit.lua"]:127: attempt to get length of field 'data' (a nil value)
stack traceback:
[love "boot.lua"]:345: in function '__len'
[string "edit.lua"]:127: in function 'invalid1'
[string "edit.lua"]:116: in function 'check_locs'
[string "run.lua"]:35: in function 'initialize'
main.lua:96: in function 'initialize'
[string "app.lua"]:144: in function 'run_tests_and_initialize'
[string "app.lua"]:16: in function <[string "app.lua"]:13>
[C]: in function 'xpcall'
[love "boot.lua"]:361: in function <[love "boot.lua"]:348>
[C]: in function 'xpcall'
This works better on mobile platforms while seeming about as useful
anywhere else.
I've verified that anyone who already edited a file will continue to use
its path from settings.
scenario:
press ctrl+f, type in a string
hit down arrow if needed until the screen scrolls
press enter
click with the mouse somewhere
Before this commit the app would crash because cursor was above screen
top.
I've been noticing in pensieve.love in particular that once a month or
so I lose data if I quit immediately after typing in something. Nothing
major, just the odd link between notes which leaves things in an
inconsistent state. Let's see if this helps.
Scenario: make some edits, select some text, make some more edits. Press
ctrl-z.
Before this commit, undo would stop at the point of selection and
previous edits would become unreachable.
After this commit, both ctrl-z and ctrl-y seem able to span the point of
selection.
Not directly relevant here, but forks of this project that permit
zooming can run into weird glitches if margins are not a whole number of
pixels.
I'd always assumed a type system that divided ints into floats was
strictly superior, but now I have experienced a situation where
requiring ints isn't just a compromise for the underlying CPU
implementation. Particularly since Lua's print() silently hides really
tiny fractions.
We had a regression since commit 60e1023f0 on Nov 27. Turns out we do
need the ancient hack after all.
But no, we won't go back to the hack. It's a simple problem to fix
right. And while we're at it, we'll fix the test harness to be more
realistic so it would have caught this problem.
This is compatible with Javascript, and it also seems like a better
default; when people forget to think about return values in click
handlers, they should be consumed.