Open bibimbop opened 11 years ago
This is basically a duplicate of #113, I am closing it. See #113 for comments.
On second thought, it shouldn't be a dup. #113 was mostly fixed. I had forgotten all that I did for reflow before.
There is a fundamental issue with Qt's model of undo/redo for text documents, which causes the error noted here and in #113 and #115. The issue is that the QTextDocument does not include updates to QTextCursors in its undo command stack. To explore the issue, experiment with this simple example program: (https://dl.dropboxusercontent.com/u/320395/bookmarks.py)
The Qt Undo/Redo model is described here: (http://qt-project.org/doc/qt-4.8/qundo.html). In general any action applied to the data model is to be recorded as a QUndoCommand and pushed onto a QUndoStack. To Undo, the command is popped from the stack and applied in reverse. In the QTextDocument model, the only commands recorded are deletions and insertions. (Replacements are encoded as a deletion followed by an insertion.) To undo a delete, the deleted text is inserted; to undo an insert, delete the inserted text. And this will in fact restore the original text.
However, Qt allows for any number of QTextCursors to be associated with a QTextDocument. Each is simply two integers, a position and an anchor. If the integers are different they define a selection, if the same, a position. Whenever it deletes or inserts text, QTextDocument runs through all associated QTextCursors and updates them by adding or subtracting from their position and anchor values. The results can be explored with the above example program.
Qt apparently does NOT record these cursor updates as part of its Undo stack. Instead it relies upon the natural effect of inserts and deletes to bring the anchors back to their original positions. And this is what happens when the changed text is outside the span of the cursor's selection. However, if the changed text overlaps on a cursor (or cursors) the undo operation will not restore the cursor to its original position.
PPQT records page boundaries and user bookmarks as QTextCursors. In the case of issue #113, the Reflow operation replaced each paragraph whole. Whenever a page or bookmark cursor pointed within the paragraph, the result of Reflow was to push it to the end of the replaced text, i.e. end of the paragraph. Because I had full control of the text during Reflow, I could fix this by inserting Zero-width spaces before Reflow, and after Reflow, finding them and using them to reconstruct the page-marker cursors. (NOTE: that fix to #113 does not fix a user bookmark cursor, which will still get displaced to the end of its paragraph.)
Issue #115 covers the much more general case of regex find/replace. Here virtually any amount of text might be selected and replaced, and any QTextCursor that points within that span will be displaced to the end of the replaced string. This cannot be fixed by inserting markers before and removing them after, as for #113, because the inserted marker could make the regex find fail; or if not, it would be deleted during the replacement anyway.
Issue #147 is the same issue in a different guise. Here, the Undo operation reverses each change made by Reflow, that is, for each paragraph in the document it replaces the new paragraph text with the old paragraph text. Wherever that replacement spans a page or bookmark position, the cursor is displaced to the end of the replacement string. (Oddly it seems to displace them to the front of the new text instead of the end but it's the same problem.) Again this can't be fixed by inserting something in the paragraph because we don't get control to do that, or to fix the cursors afterward. Undo is completely opaque, within the C++ code of QTextDocument.
There are only two possibilities. One, to find a different way to record page and bookmark positions than with QTextCursors, and Two, to get Qt to do what it should, namely, include QTextCursor updates as part of every QUndoCommand within QTextDocument. I am going to file a bug for the latter but have little hope of seeing it fixed. For the former, I'm open to suggestions.
Pleased to note that after I added a screencast demonstrating the problem (http://www.youtube.com/watch?v=XIs2QkTlOxI) the above bug was upgraded from "not verified" to "Important" status. Woo-hoo!
According to #113, undo doesn't put the page markers where they were. This deserves an entry for itself.
To reproduce:
The document remains unchanged (expected), however the metafile is full of changes.
That bug is very easy to hit: