Don't crash/exit on errors in keybindings

[bjornbm]

I think ple shouldn't exit when it encounters an error in an keybinding (or other areas that the editor api allow to be tweaked by ple_init). Since Lua has little safety, a small typo in a seldom-used corner of one's init file could lead to the loss of all “writing” done since last save, which could be extremely annoying. For a real world case, using my old ple_init (from when the buffer was still exposed) loads OK in PLE, but crashes to my shell prompt when I use a keybinding function using b, with the following stack trace:

^K                                       /Users/bjorn/.config/ple/ple_init.lua:20: attempt to index a nil value (local 'b')
stack traceback:
	/Users/bjorn/.config/ple/ple_init.lua:20: in function 'emacs_ctlk'
	/Users/bjorn/repos/ple/ple.lua:1085: in function </Users/bjorn/repos/ple/ple.lua:1070>
	[C]: in function 'xpcall'
	/Users/bjorn/repos/ple/ple.lua:1113: in local 'main'
	/Users/bjorn/repos/ple/ple.lua:1127: in main chunk
	[C]: in function 'dofile'
	/Users/bjorn/bin/ple:5: in main chunk
	[C]: in ?

I think ple should catch the error and display it it, but then return to the editor. Probably with a recommendation to save ASAP and restart PLE since the erroring function may have messed up state? Maybe even provide a prompt with three options:

1. Save the buffer and exit.
2. Write the current buffer contents to another file (add .saved to file name?) and exit. This is to preserve the last saved version and let the user do a manual merge. This is in case the user is worried the erroring function may have messed up the buffer state before the error, but they don't want to discard it completely and lose their work since last save.
3. Continue (assume nothing is corrupted by the error and go on)

[philanc]

Good point. I'd rather implement a simple (simplistic?) behavior in case of error, but allow the user to define a more sophisticated approach (such as what you suggest) in their ple_init.lua.

In my commit 53df746, I added editor.error_handler. On error, it is invoked with the Lua stack traceback.

You may want to have a look at editor_loop() in ple.lua. The default error handler is defined just above. It switches to buffer OUT and display a short message and the stack traceback. It doesn't exit the editor, so it leaves the user an opportunity to save modified buffers.

Now we still lack exported functions (i.e. functions available in editor) to iterate buffers and test if a buffer is modified.

So your suggested approach still cannot be fully implemented in ple_init.lua, but at least this is a first step!

You can test it with the last ple_init.lua: a new binding ^X-t has been added to provoke an error and test the handler.

[bjornbm]

No objectives to keeping it simplistic and leaving the recovery to the user.

In my commit 53df746, I added editor.error_handler. On error, it is invoked with the Lua stack traceback.

You may want to have a look at editor_loop() in ple.lua. The default error handler is defined just above. It switches to buffer OUT and display a short message and the stack traceback. It doesn't exit the editor, so it leaves the user an opportunity to save modified buffers.

Hmm, switching to a new buffer with the error message as its contents confused me a bit (before I reread your message). I'm not sure this is behavior a unfamiliar user would expect… it also behaves a bit weird when a second error is triggered (in the OUT buffer).

I would have expected something more like a multiline e.msg (in red would be nice), but haven't dug into plterm to know how easy that is. But in terms of a minimal acceptable UX, I think you could just append the message to the end of the screen and let the terminal scroll (assuming this is straightforward), then wait for a keypress and redraw (as ^L).

(I can a use-case for the behavior you implemented, but don't think it should be the default. A user could have it as their custom behavior in their ple_init.)

[philanc]

I see your point.
Contrary to the real Emacs, in ple, the "minibuffer" (ie. the screen area for simple messages and prompt) is not a real buffer -- just a single line.

I don't want to develop multi-line pop-up messages (and implement the associated complexity: framing, scrolling, redisplay...).

I think that for ple, if a multi-line text has to be displayed, the right place to do it is in a regular buffer - thus the proposed *OUT* buffer solution.

A maybe simpler, "less disorientating" solution might be to copy the traceback to the *OUT* buffer and display a single line msg with something like "Error! (details available in the *OUT* buffer)".

Or maybe even omit the "detail available..." part which is probably meaningless for "unfamiliar users" and could be documented for "extension-writing" users.

[bjornbm]

Understood and agree with keeping it simple.

I was thinking of something really simplistic like b91c046.

I don't know if I'm using plterm correctly… all I tell is it seems to work OK in my terminal.

As I mentioned in the commit message, it would be nicer if the editor text did not scroll and cursor location
did not change when showing the error message. The scrolling and, in particular, cursor location change means that the context will be less clear to the user while reading the error message. The good thing is of course that the cursor goes back to the good location after the redisplay.

[philanc]

Yes it works ok.

it would be nicer if the editor text did not scroll and cursor location
did not change when showing the error message. The scrolling and, in particular, cursor location change means that the context will be less clear to the user while reading the error message.

This goes with the standard terminal behavior in sane mode. If you want direct control of what you write, where you write it and where goes the cursor, you may stay in raw mode, and directly use term and implement a function to display a multi-line string. It is not simple (actually this is the most complex part of the buffer display code!)

Just for helping with the cursor issue, you can hide the cursor with term.hide() and show it again after the user has hit space with term.show().

If you want to implement this in ple_init.lua, you have make functions visible:

local term = require "term"  -- but then it doesn't work anymore with the 'ple' bundled file
local color = term.color
local col = term.colors
local readchar = editor.readchar

[bjornbm]

Mmm, the best I think I can do in ple_init.lua without require "term" is:

editor.error_handler = function(r)
  print(
    "\r\nAN ERROR HAS OCCURRED! \r\n\r\n"
    .. "It is recommended to exit the editor\r\n"
    .. "(you may try to save your buffers first)\r\n"
    .. string.rep("_", 72) .. "\r\n"
    .. string.gsub(r, "\n", "\r\n")
  )
  editor.readchar("Press SPACE to continue", " ")
  editor.actions.redisplay()
end

Note the replacing of \n with \r\n. Biggest shortcoming is that I cannot change to color of the text, not hide the cursor (thanks for pointing that out!).

In any case, I'm quite fine with the error handling you implemented and likely won't bother overriding it. My comment was more that for someone who didn't follow this issue might find it confusing that they are now in a different buffer (as I was at first).

EDIT: see also #16

[philanc]

Haha! good try! :-)
Going just a bit further with the same approach:

editor.error_handler = function(r)
  print(
       "\027[2J"  -- clear screen (it erases some editor artifacts but not the top line...)
    ..  "\027[31;1m" -- for bright red.  replace with  "\027[31m" for dark red
    .. "\r\nAN ERROR HAS OCCURRED! \r\n\r\n"
    .. "It is recommended to exit the editor\r\n"
    .. "(you may try to save your buffers first)\r\n"
    .. string.rep("_", 72) .. "\r\n"
    .. string.gsub(r, "\n", "\r\n")
  )
  editor.readchar("Press SPACE to continue \027[?25l", " ") -- hide the cursor
  print("\027[?25h") -- show the cursor once user has pressed space
  editor.actions.redisplay()
end

Have fun with using directly ANSI sequences! :-)

[bjornbm]

Now we're talking! 😆

I'll close this, but see #17.