~akkartik/techmeet.love

b99d1680 — Kartik K. Agaram 11 hours ago main
Merge pensieve.love
c1be7266 — Kartik K. Agaram 11 hours ago
Merge lines.love
2fdb6fa7 — Kartik K. Agaram 12 hours ago
update a stale comment
1753cfba — Kartik K. Agaram 3 days ago
Merge pensieve.love
5656c2a7 — Kartik K. Agaram 3 days ago
merge bugfix

I'm now computing screen_bottom1 which assumes State.top always exists.
This invariant is violated in pensieve.love.

Continuing to hack my way in this project; it's not really intended for
others.
2a85c1fe — Kartik K. Agaram 5 days ago
Merge pensieve.love
30051768 — Kartik K. Agaram 5 days ago
Merge lines.love
55f5c2d6 — Kartik K. Agaram 5 days ago
crap, fix some final changes in the source editor
7aa43d1d — Kartik K. Agaram 5 days ago
comment
85b09772 — Kartik K. Agaram 5 days ago
whitespace
69c88da9 — Kartik K. Agaram 5 days ago
stop caching starty

This is quite useful because I used to have a long list of places in
which to invalidate the cache.
f2299cb4 — Kartik K. Agaram 5 days ago
stop caching screen_bottom1

I'm not sure this is very useful. I had an initial idea to stop using
screen_bottom1 in final_text_loc_on_screen, by starting from screen_top1
rather than screen_bottom1. But that changes the direction in which we
scan for the text line in situations where there is somehow no text on
screen (something that should never happen but I have zero confidence in
that).

Still, it doesn't seem like a bad thing to drastically reduce the
lifetime of some derived state.

Really what I need to do is throw this whole UX out and allow the cursor
to be on a drawing as a whole. So up arrow or left arrow below a drawing
would focus the whole drawing in a red border, and another up arrow and
left arrow would skip the drawing and continue upward. I think that
change to the UX will eliminate a whole class of special cases in the
code.
19615ead — Kartik K. Agaram 6 days ago
bugfix in source editor: don't clear selection on M-arrow
9b5a78d3 — Kartik K. Agaram 6 days ago
bugfix in source editor
9501f01c — Kartik K. Agaram 7 days ago
fix a crash involving mouse and drawings

Thanks Alex Schroeder for reporting this crash. The scenario:
  * Edit a file like say this repo's Readme.
  * The second line is empty and there's a '+' to insert a drawing.
    Click on that.
  * Resize the window so just the first line of text and the drawing are
    visible.
  * Close the window.
  * Reopen lines.love, it will reopen the same file.
  * Click on the left margin to the left of the drawing.

Before this commit these steps yielded the following crash:

  Error: bad argument #1 to 'len' (string expected, got nil)
  text.lua:626: in function 'pos_at_end_of_screen_line'
  edit.lua:298: in function 'mouse_press'

There were two distinct problems here:

1. State.screen_bottom1 is not required to point to a text line, it
   could just as well be a drawing. I have been sloppy in handling that.
2. The bug was partially masked (the need to close and reopen the
   window) by a second bug: inserting a drawing was not invalidating the
   cache I save of starty coordinates for each line. (I've inserted and
   deleted starty invalidations a few times in the past, but it looks
   like I'd never had one in this particular location edit.draw before.)

How did these issues get missed for years?
  - Even though I use lines.love on a daily basis, it turns out I don't
    actually create line drawings all that often.
  - When I do, I'm still living in files that are mostly text with only
    an occasional drawing.
  - I keep my windows fairly large.

Between these 3 patterns, the odds of running into a drawing as the
first or bottom-most line on the screen were fairly small. And then I
had to interact with it. I suspect I tend to interact with drawings
after centering them vertically.

---

Bug #1 in particular has some interesting past history.

* Near the start of the project, when I implemented line-wrapping I
  started saving screen_bottom, the bottom-most line displayed on
  screen. I did this so I could scroll down easily just by assigning
  `screen_top = screen_bottom`. (On the other hand, scrolling up still
  required some work. I should perhaps get rid of it and just compute
  scrolls from scratch each time.)

* Also near the start of the project, I supported selecting text by a
  complex state machine spanning keypress, mouse press and mouse
  release:
    mouse click (press and immediate release) moves cursor
    mouse drag (press and much later release) creates selection
    shift-click selects from current cursor to click location
    shift-movement creates/grows a selection

* On 2023-06-01, inscript reported a bug. Opening a window with just a
  little bit of text (lots of unused space in the window), selecting all
  the text and then clicking below all the text would crash the editor.

  To fix this I added code at the bottom of edit.mouse_press which
  computed the final visible line+pos location and used that in the
  cursor-move/text-selection state machine. It did this computation
  based on.. screen_bottom. But I didn't notice that screen_bottom could
  be a drawing (which has no pos). This commit's bug/regression was
  created.

* On 2023-09-20, Matt Wynne encountered a crash which got me to realize
  I need code at the bottom of edit.mouse_release symmetric to the code
  at the bottom of edit.mouse_press. I still didn't notice that
  screen_bottom could be a drawing.

So in fixing inscript's bug report, I introduced (at least) 2
regressions, because I either had no idea or quickly forgot that
screen_bottom could point at a drawing.

While I created regressions, the underlying mental bug feels new. I just
never focused on the fact that screen_bottom could point at a drawing.

This past history makes me suspicious of my mouse_press/mouse_release
code. I think I'm going to get rid of screen_bottom entirely as a
concept. I'll still have to be careful though about the remaining
locations and which of them are allowed to point at drawings:

  - cursor and selection are not allowed to point at drawings
  - screen_top and screen_bottom are allowed to point at drawings

I sometimes copy between these 4 location variables. Auditing shows no
gaps where cursor could ever end up pointing at a drawing. It's just
when I started using screen_bottom for a whole new purpose (in
the mouse_press/release state machine) that I went wrong.

I should also try getting rid of starty entirely. Is it _really_ needed
for a responsive editor? I think I introduced it back when I didn't know
what I was doing with LÖVE and was profligately creating text objects
willy-nilly just to compute widths.

Getting rid of these two fairly global bits of mutable state will
hopefully make lines much more robust when the next person tries it out
in 6 months :-/ X-(

Thanks everyone for the conversation around this bug:
  https://merveilles.town/@akkartik/112567862542495637

---

Bug #2 has some complexity as well, and might lead to some follow-on
cleanup.

When I click on the button to insert a new drawing, the mouse_release
hook triggers and moves the cursor below the new drawing. This is
desirable, but I'd never noticed this happy accident. It stops working
when I invalidate starty for all lines (which gets recomputed and cached
for all visible lines on every frame).

Fixing this caused a couple of unit tests start crashing for 2 reasons
that required their own minor fixes:

  - My emulated mouse press and release didn't have an intervening
    frame and so mouse_release no longer receives starty. Now I've added
    a call to edit.draw() between press and release.

    This might actually bite someone for real someday, if they're
    running on a slow computer or something like that. I've tried to
    click really fast but I can't seem to put mouse_press and release in
    the same frame (assuming 30 frames per second)

  - My tests' window dimensions often violate my constraint that the
    screen always have one line of text for showing the cursor. They're
    unrealistically small or have a really wide aspect ratio (width 2x
    of height). I suspect lines.love will itself crash in those
    situations, but hopefully they're unrealistic. Hmm, I wonder what
    would happen if someone maximized in a 16:9 screen, that's almost
    2x.. Anyways, I've cleaned a couple of tests up, but might need to
    fix up others at some point. I'd have to rejigger all my brittle
    line-wrapping tests if I modify the screen width :-/ X-(
72a27cf8 — Kartik K. Agaram 27 days ago
Merge pensieve.love
472a1076 — Kartik K. Agaram 27 days ago
Merge lines.love
3e0cb2ed — Kartik K. Agaram 27 days ago
document recent handlers
e29c2358 — Kartik K. Agaram 27 days ago
Merge pensieve.love
4e15b8ee — Kartik K. Agaram 27 days ago
Merge lines.love
Next