scenario: run without config file, quit, run again
expected: font size remains the same on second run
Before this commit it was increasing on each run.
It turns out the font height that you pass into love.graphics.newFont()
is not the result of font:getHeight().
We'll end up calling Text.redraw_all anyway, which will clear starty and
much more besides.
We'll still conservatively continue clearing starty in a few places
where there's a possibility that Text.redraw_all may not be called. This
change is riskier than most.
Just checking mouse.isDown works if the editor is the entirety of the
app, as is true in this fork. However, we often want to introduce other
widgets. We'd like tapping on them to not cause the selection to flash:
https://news.ycombinator.com/context?id=38404923&submission=38397715
The right architecture to enforce this is: have each layer of the UI
maintain its own state machine between mouse_press and mouse_release
events. And only check the state machine in the next level down rather
than lower layers or the bottommost layer of raw LÖVE.
Each one should provide a message that will show up within LÖVE. Stop
relying on nearby prints to the terminal.
I also found some unnecessary ones.
There is some potential here for performance regressions: the format()
calls will trigger whether or not the assertion fails, and cause
allocations. So far Lua's GC seems good enough to manage the load even
with Moby Dick, even in some situations that caused issues in the past
like undo.
Scenario:
* select a line or two
* delete
* press C-z to undo. The lines return and are highlighted.
* press C-y to redo.
Before this commit the editor was crashing:
Error
edit.lua:169: assertion failed!
Traceback
[love "callbacks.lua"]:228: in function 'handler'
[C]: in function 'assert'
edit.lua:169: in function 'draw'
text.lua:1064: in function 'cursor_out_of_screen'
edit.lua:487: in function 'keychord_press'
run.lua:842: in function 'keychord_press'
main.lua:215: in function 'keychord_press'
keychord.lua:11: in function <keychord.lua:5>
app.lua:31: in function <app.lua:22>
[C]: in function 'xpcall'
I was forgetting a step in C-y that was done in C-z. Now undo and redo
are symmetric.
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.
Scenario: press C-f and search for something.
Some of the time we get a crash with this call stack:
Error
text.lua:970: attempt to compare nil with number
Traceback
[love "callbacks.lua"]:228: in function 'handler'
text.lua:970: in function 'lt1'
search.lua:72: in function 'search_next'
run.lua:985: in function 'search_next_in_pane'
run.lua:939: in function 'search_next'
run.lua:784: in function 'text_input'
main.lua:237: in function <main.lua:230>
app.lua:31: in function <app.lua:22>
[C]: in function 'xpcall'
Not fixed yet.
The cause: plan_draw is conservative and sometimes tries to render panes
that never overlap the viewport.
This bug was exacerbated by fixing the inscript bug which started
setting screen_bottom1 to nil. But the problem existed before as well,
we just operated on stale screen_bottom1 locations.
This bug doesn't generalize to many other scenarios. It isn't a problem
for forks without the surface metaphor, though it might affect
driver.love as well. Even in pensieve.love, the only place that uses
screen_bottom while spanning multiple editors is find across the entire
surface.
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.
loc.pos should never be computed based on #line.data
scenario:
- open a surface containing some unicode
- search for a rare pattern that requires scanning over entire surface
including the unicode
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.
Kartik K. Agaram