Saturday, November 29, 2014

Early Morning Obsessions

So yesterday a user of PPQT V1 found a real bug, the first actual "this is a coding error that ought to be fixed" bug since I stopped development many months ago. It's something in the ASCII reflow code. Under a particular setting of parameters, reflowing a poem produces a stack trace,

Traceback (most recent call last):
  File "/Users/original/Desktop/scratch/build/ppqt/out00-PYZ.pyz/pqFlow", line 374, in reflowDocument
  File "/Users/original/Desktop/scratch/build/ppqt/out00-PYZ.pyz/pqFlow", line 591, in theRealReflow
  File "/Users/original/Desktop/scratch/build/ppqt/out00-PYZ.pyz/pqFlow", line 1350, in optimalWrap
IndexError: list index out of range

So it is something in the rather complex Knuth-Pratt optimal rewrap logic. I haven't looked at the code yet to see what the problem really is, nor have I made up my mind what to do about fixing it. There appears to be a not-too-awkward work-around so maybe I'll do nothing. I really do not want to have to rebuild the distribution bundles for V1. It would probably be doable. I just don't want to do it.

That's one of the thoughts I'm having, here at 6am in Honolulu, where we are visiting for the Thanksgiving weekend, instead of sleeping for another hour. Lying in bed obsessing about PPQT instead of sleeping.

But there's another and more serious thought, and that is about Qt and Edit Blocks, sometimes called edit macros. If you want to make a series of edit actions undoable with a single Undo, you create an edit block:

    work_tc = QTextCursor(my_edit_document)
    work_tc.beginEditBlock()
    # change the document in many ways via
    # work_tc, often in a loop
    work_tc.endEditBlock()

It works quite well, as long as the endEditBlock() call is executed. Well, why would it not? It would not, if somewhere between the Begin and the End your program raises an un-caught exception like "list index out of range" not inside a try-except block.

That's what is happening above. Reflowing the text is done inside an edit block, and because of the exception, the block is never ended.

Normal Python programs, when they cause a stack trace like this, simply terminate. But not a Qt app! The top function in the stack trace, reflowDocument(), was called by the QApplication as a result of processing an event, in fact the event of clicking on the "Reflow Document" button. The QApplication doesn't care that this subroutine ended with an exception. It ended, that's all. The QApplication keeps running, processing other events, calling other methods of the various objects to respond to button clicks and menu choices and edit keystrokes.

I really don't know what happens to the open Undo block. I presume the variables created by reflowDocument() go out of scope. What happens when a QTextCursor with an open Undo Block goes out of scope and is garbage-collected? The user reports that the operation cannot be undone.

In fact the whole document is in an ambiguous state and probably should not be saved. Maybe it is alright but maybe there is some garbage in it, or some text is missing (because the reflow logic deleted it and had not put it back when the error occurred). But if the user calls Quit, there will be a prompt to save the modified document.

The very important lesson here is: An Undo Block should never be opened unless it can be guaranteed to close. The situation is just like modifying a file: the logic should always be

    work_tc = QTextCursor(my_edit_document)
    work_tc.beginEditBlock()
    try:
        # change the document in many ways via
        # work_tc, often in a loop
    finally:
        work_tc.endEditBlock()

Thus the Undo Block will always be closed no matter what goes wrong.

I did not do this at any of the several points where I have Undo Blocks in V1. And I realize (here in the dark, not sleeping on a Saturday morning in Hawaii) that I just wrote the first use of an Undo Block in V2, in the footnote code, and I did not do it there. So that's bad. I need to fix that.

But what shall I do about this V1 problem? I will probably have to try to fix it and make new distribution bundles. You cannot imagine my reluctance to do so.

No comments: