commits
This reverts commit 5d48994aa9b6e45804a090c2f5e6e4444c8cd628.
The issue was just invalid Unicode. No bug in my editor, technically.
There is still an open question of how to edit non-Unicode. I need a
pure-ASCII mode or something.
Multi-screen apps seem to have a different protocol. I'm not going to
bother to add a call stack just yet.
I didn't realize until now that:
loading some code and calling the resulting thunk shows a call stack on error
but pcall/xpcall on the resulting thunk does not by default
Perhaps this is not a good idea. Adding the call stack makes the error
message much noisier, and people will have to learn how to read it. But
it's useful in advanced situations, and those who aren't doing advanced
things will hopefully focus on just the first line.
Now we also stop stealing live.eval, which is designed to work in a very
different context: live editing using driver.love.
Scenario:
defs/ contains a global variable with an error, say arithmetic with nil
Before this commit, the sequence of events:
love.run calls xpcall(love.load, live.handle_initialization_error)
love.load calls live.load
which calls live.load_files_so_far()
which calls live.eval
which throws an error
which gets caught by live.handle_initialization_error
which sets run_frame = initialization_error_run_frame
now love.run now starts calling xpcall(initialization_error_run_frame, live.handle_error)
initialization_error_run_frame tries to perform arithmetic on Live.previous_read
but Live.previous_read is not set because we never got to that point in love.load
so we call live.handle_error, which tries to read Live.previous_read.. ad infinitum.
Our error handling flows are getting quite hairy.
love.run
xpcall(love.load, live.handle_initialization_error)
xpcall(love.run, live.handle_error)
live.handle_initialization_error
xpcall(love.load, live.handle_initialization_error)
run_frame = initialization_error_run_frame, therefore xpcall(initialization_error_run_frame, live.handle_error)
live.handle_error
run_frame = error_run_frame, therefore xpcall(error_run_frame, live.handle_error)
The root problem is there is no way to guarantee the error handler in
xpcall won't itself throw an error. And our attempts to provide a good
experience mean error handlers aren't trivial. The best I can do is try
to ensure errors in error handlers happen seldom. This commit shaves off
one such case: try to ensure Live.previous_read is set before any error
handlers get an opportunity to run.
A user script (nanochat) sees more Unicode in the wild and has been
crashing Carousel since my bugfix of commit 0ba795ebafa. I don't
understand yet why my_utf8.chars with that one line uncommented doesn't
have the same semantics as wrap.internal.utf8chars.
If I see something weird on screen, I can just type in right below it,
set edit.debug_wrap to the line number I want to inspect line wrap for
and run.
I see a single set of prints in the lower half showing me what's
happening.
Any fix will need to happen upstream and flow down to this fork. But
Carousel makes for a convenient debug UI, particularly since the narrow
width of a phone is more of a stress test for line wrap.
I'd never noticed until now that utf8.codes returns byte indexes rather
than codepoint indexes. Bring back our old implementation of utf8chars.
Carousel's browser is still out of sync, using the older implementation
with should_word_wrap.
This seems clearer at the cost of 150 additional lines.
If the indent is too much we'd rather make better use of vertical space.
I noticed now because the new word wrap algorithm was failing a new
assertion.
The problem with the previous version: it was possible for a word that
was just the right length to leave a short line:
- a long word fits in current line
- the word+trailing spaces don't fit in current line
In that situation it feels like the right thing to do is to give up on
word wrap. Put a lot of the long word in the first line, then wrap a
little before we get to spaces.
How much before? Well, a single letter wrapping feels ugly as well. I
don't really have much sense beyond that. But since we already have a
hacky threshold of 0.8*line width, let's just use that here as well.
Once a line is no longer considered too short we immediately truncate the
current word if continuing it would cause the next line to start with
whitespace.
To accomplish this I had to reorganize things. The should_word_wrap
helper has to go because we also need intelligence to truncate words
when wrapping.
For a brief shining moment I had tests because I had a side-effect-free
component. But now it's gone and the wrapping again needs font widths.
So, once again I choose no tests over brittle tests.
This is much simpler than what was in the original lines.love
All it can do is test side-effect-free functions.
I don't want to go back down the road of being sensitive to the current
font.
scenario:
on Mac OS, make a selection spanning a line boundary
press ctrl+c rather than cmd+c which would copy
Before this commit the app would crash:
Error: edit.lua:212: bad argument #1 to 'setColor' (number expected, got nil)
stack traceback:
[C]: in function 'setColor'
edit.lua:212: in function 'draw'
draw_editors:12: in function 'draw_editors'
on.draw:25: in function 'draw'
main.lua:121: in function 'draw'
app.lua:56: in function <app.lua:36>
[C]: in function 'xpcall'
app.lua:29: in function <app.lua:28>
[C]: in function 'xpcall'
[love "boot.lua"]:377: in function <[love "boot.lua"]:344>
The problem was that we were deleting the selection, but then not
going into any of the cases in the keychord handler that would trigger
recomputation of colors for the buffer. And so editor.colors went out of
sync with editor.lines
I only used this word because it was longer than 'ok' and so presented a
larger hit area. But it's always grated on me. It sounds too
bureaucratic. Nobody should have to submit to Carousel. It's not like
a lot of other software out there.
Another issue: the 'load' and 'save' buttons are close by on the menu.
If you fat-fingered the wrong one the consequences could be
catastrophic. Now we give some feedback on which one you tapped.
https://love2d.org/wiki/Transform
Now if a screen uses transforms, that doesn't cause Carousel's menus to
be drawn in the wrong place (though it is quite a fun bug).
I didn't realize until now that:
loading some code and calling the resulting thunk shows a call stack on error
but pcall/xpcall on the resulting thunk does not by default
Perhaps this is not a good idea. Adding the call stack makes the error
message much noisier, and people will have to learn how to read it. But
it's useful in advanced situations, and those who aren't doing advanced
things will hopefully focus on just the first line.
Now we also stop stealing live.eval, which is designed to work in a very
different context: live editing using driver.love.
Scenario:
defs/ contains a global variable with an error, say arithmetic with nil
Before this commit, the sequence of events:
love.run calls xpcall(love.load, live.handle_initialization_error)
love.load calls live.load
which calls live.load_files_so_far()
which calls live.eval
which throws an error
which gets caught by live.handle_initialization_error
which sets run_frame = initialization_error_run_frame
now love.run now starts calling xpcall(initialization_error_run_frame, live.handle_error)
initialization_error_run_frame tries to perform arithmetic on Live.previous_read
but Live.previous_read is not set because we never got to that point in love.load
so we call live.handle_error, which tries to read Live.previous_read.. ad infinitum.
Our error handling flows are getting quite hairy.
love.run
xpcall(love.load, live.handle_initialization_error)
xpcall(love.run, live.handle_error)
live.handle_initialization_error
xpcall(love.load, live.handle_initialization_error)
run_frame = initialization_error_run_frame, therefore xpcall(initialization_error_run_frame, live.handle_error)
live.handle_error
run_frame = error_run_frame, therefore xpcall(error_run_frame, live.handle_error)
The root problem is there is no way to guarantee the error handler in
xpcall won't itself throw an error. And our attempts to provide a good
experience mean error handlers aren't trivial. The best I can do is try
to ensure errors in error handlers happen seldom. This commit shaves off
one such case: try to ensure Live.previous_read is set before any error
handlers get an opportunity to run.
If I see something weird on screen, I can just type in right below it,
set edit.debug_wrap to the line number I want to inspect line wrap for
and run.
I see a single set of prints in the lower half showing me what's
happening.
Any fix will need to happen upstream and flow down to this fork. But
Carousel makes for a convenient debug UI, particularly since the narrow
width of a phone is more of a stress test for line wrap.
The problem with the previous version: it was possible for a word that
was just the right length to leave a short line:
- a long word fits in current line
- the word+trailing spaces don't fit in current line
In that situation it feels like the right thing to do is to give up on
word wrap. Put a lot of the long word in the first line, then wrap a
little before we get to spaces.
How much before? Well, a single letter wrapping feels ugly as well. I
don't really have much sense beyond that. But since we already have a
hacky threshold of 0.8*line width, let's just use that here as well.
Once a line is no longer considered too short we immediately truncate the
current word if continuing it would cause the next line to start with
whitespace.
To accomplish this I had to reorganize things. The should_word_wrap
helper has to go because we also need intelligence to truncate words
when wrapping.
For a brief shining moment I had tests because I had a side-effect-free
component. But now it's gone and the wrapping again needs font widths.
So, once again I choose no tests over brittle tests.
scenario:
on Mac OS, make a selection spanning a line boundary
press ctrl+c rather than cmd+c which would copy
Before this commit the app would crash:
Error: edit.lua:212: bad argument #1 to 'setColor' (number expected, got nil)
stack traceback:
[C]: in function 'setColor'
edit.lua:212: in function 'draw'
draw_editors:12: in function 'draw_editors'
on.draw:25: in function 'draw'
main.lua:121: in function 'draw'
app.lua:56: in function <app.lua:36>
[C]: in function 'xpcall'
app.lua:29: in function <app.lua:28>
[C]: in function 'xpcall'
[love "boot.lua"]:377: in function <[love "boot.lua"]:344>
The problem was that we were deleting the selection, but then not
going into any of the cases in the keychord handler that would trigger
recomputation of colors for the buffer. And so editor.colors went out of
sync with editor.lines
I only used this word because it was longer than 'ok' and so presented a
larger hit area. But it's always grated on me. It sounds too
bureaucratic. Nobody should have to submit to Carousel. It's not like
a lot of other software out there.
Another issue: the 'load' and 'save' buttons are close by on the menu.
If you fat-fingered the wrong one the consequences could be
catastrophic. Now we give some feedback on which one you tapped.