[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: questions regarding QTextDocument behaviour
From:       Pierre Stirnweiss <pierre.stirnweiss () t-online ! de>
Date:       2008-11-29 18:34:08
Message-ID: 200811291934.08861.pierre.stirnweiss () t-online ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Saturday 29 November 2008 17:16:51 Florian wrote:
> Am Samstag 29 November 2008 12:29:15 schrieb Pierre Stirnweiss:
> > Hi,
>
> Hi Pierre,
>
> > In order to participate in the common push towards stable Koffice 2.0, I
> > am looking into the undo/redo of the TextShape.
>
> I'm afraid I can't help you much with this. I've been looking at the
> undo/redo code in the TextShape recently, too. Mainly because I need to
> support undo/redo with the ParagraphTool and I needed a starting point for
> this. Unfortunately I didn't get very far, because I simply don't have
> enough time for this right now, but it's good to know whom to ask if I ever
> need help ;-)

I will be working on this if no one has an objection. I will keep the list 
posted on what I find out and what I intend to do.


> >
> > Shouldn't there be consistency between the cases?
>
> I think it would make sense to create separate commands for koffice and
> rocks in all cases.

Yes, this is my basic idea. However, the catch we are running into is the 
following:

As a background info:
The QTextDocument has its own internal undo/redo stack. It creates commands 
for every editing action and merge them with its own rules (as I have 
described above). I haven't found any hooks to influence the QTextDocument in 
this behaviour.

So we are now at a crossing in path, in what I think originally was planned 
for: 

1) Either we rely on QTextDocument internals:

So as long as we pop as many QUndoCommand onto the canvas stack as there are 
"QTextDocument internal" undo command, things will be kept in sync. This is 
achievable as (partially) done in the TextTool by connecting the 
undoCommandAdded signal from the QTextDocument to a slot creating an 
undoCommand to be pushed to the stack.
The problem with this method is that when a change that is normally perceived 
as singular is in fact  non "atomic", an excessive (from a user perspective) 
number of undo are created. 
For an example: when pressing enter (new paragraph), the current 
implementation calls for insertBlock (1st edit action), then for setFormat 
with the style (2nd edit action, then again setFormat for 
TextProgressDirection (3rd edit action). This leads to 3 edit actions as far 
as QTextDocument is concerned. From the user perspective, he would have to 
press twice on undo button with no visible actions (the two formatting) and 
only on the third press, the carriage return would be removed. This could 
probably be solved by setting the style first and call insertBlock with the 
style as argument, but that might not be true for all edit actions.

2) Or, we do not use the internal edits and undoStack of the QTextDocument. 
This can be achieved by having a QUndoStack, create subclassed QUndoCommands 
and push them on the stack. By reimplementing the redo and undo functions to 
edit/unedit the document, all would be fine: we would have a fine and consistent 
control on what is done/undone. The problem there is that we would have to 
keep track of everything (text entered, position, format,...). Since 
QTextDocument is doing it internally, it seems like a waste.

3) This is were KOffice lies; a bit in between.
KOffice's own action (subclassed from QUndoCommand) are making the edits on the 
document outside the undo/redo methods. These are merely calling the 
QTextDocument::undo/redo. This allows to use the QTextDocument traking of 
things which seems logical. On the other hand, it creates only one UndoCommand 
(with the startMacro/stopMacro mechanism) were, from a user perspective, one 
edit action has been done (see example of carriage return). This is how you 
arrive at situations were the QTextDocument created 3 undoCommands and the 
TextShape only one: when pressing undo once, the call for QTextDocumment::undo 
will actually only undo a part of the whole actions.


I have several ideas to solve the out-of-sync problem. However, before going 
too far in one direction, it would be better to know whether Qt is faulty or 
if its behavior was planned like that.

Pierre

[Attachment #5 (text/html)]

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" \
"http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" \
content="1" /><style type="text/css"> p, li { white-space: pre-wrap; }
</style></head><body style=" font-family:'Sans Serif'; font-size:10pt; \
font-weight:400; font-style:normal;"> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">On Saturday 29 November 2008 17:16:51 Florian wrote:</p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; Am Samstag 29 November \
2008 12:29:15 schrieb Pierre Stirnweiss:</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt; &gt; Hi,</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt;</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt; Hi Pierre,</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt;</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt; &gt; In order to participate in the common \
push towards stable Koffice 2.0, I</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">&gt; &gt; am looking into the undo/redo of the TextShape.</p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt;</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; I'm afraid I can't help \
you much with this. I've been looking at the</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt; undo/redo code in the TextShape recently, \
too. Mainly because I need to</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">&gt; support undo/redo with the ParagraphTool and I needed a \
starting point for</p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; this. \
Unfortunately I didn't get very far, because I simply don't have</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; enough time for this \
right now, but it's good to know whom to ask if I ever</p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">&gt; need help ;-)</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">I will be working on this if \
no one has an objection. I will keep the list posted on what I find out and what I \
intend to do.</p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"></p> <p style="-qt-paragraph-type:empty; \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; &gt;</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; &gt; Shouldn't there be \
consistency between the cases?</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">&gt;</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">&gt; I think it would make sense to create separate commands for \
koffice and</p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">&gt; rocks \
in all cases.</p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">Yes, this is my basic idea. However, the catch we \
are running into is the following:</p> <p style="-qt-paragraph-type:empty; \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">As a background info:</p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">The QTextDocument has its own \
internal undo/redo stack. It creates commands for every editing action and merge them \
with its own rules (as I have described above). I haven't found any hooks to \
influence the QTextDocument in this behaviour.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">So we are now at a crossing \
in path, in what I think originally was planned for: </p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">1) Either we rely on \
QTextDocument internals:</p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">So as long as we pop as many QUndoCommand onto \
the canvas stack as there are "QTextDocument internal" undo command, things will be \
kept in sync. This is achievable as (partially) done in the TextTool by connecting \
the undoCommandAdded signal from the QTextDocument to a slot creating an undoCommand \
to be pushed to the stack.</p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">The problem with this method is that when a change that is \
normally perceived as singular is in fact  non "atomic", an excessive (from a user \
perspective) number of undo are created. </p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">For an example: when pressing enter (new \
paragraph), the current implementation calls for insertBlock (1st edit action), then \
for setFormat with the style (2nd edit action, then again setFormat for \
TextProgressDirection (3rd edit action). This leads to 3 edit actions as far as \
QTextDocument is concerned. From the user perspective, he would have to press twice \
on undo button with no visible actions (the two formatting) and only on the third \
press, the carriage return would be removed. This could probably be solved by setting \
the style first and call insertBlock with the style as argument, but that might not \
be true for all edit actions.</p> <p style="-qt-paragraph-type:empty; margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;"></p> <p style=" margin-top:0px; \
margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; \
text-indent:0px; -qt-user-state:0;">2) Or, we do not use the internal edits and \
undoStack of the QTextDocument. This can be achieved by having a QUndoStack, create \
subclassed QUndoCommands and push them on the stack. By reimplementing the redo and \
undo functions to edit/unedit the document, all would be fine: we would have a fine \
and consistent control on what is done/undone. The problem there is that we would \
have to keep track of everything (text entered, position, format,...). Since \
QTextDocument is doing it internally, it seems like a waste.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">3) This is were KOffice lies; \
a bit in between.</p> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">KOffice's \
own action (subclassed from QUndoCommand) are making the edits on the document \
outside the undo/redo methods. These are merely calling the QTextDocument::undo/redo. \
This allows to use the QTextDocument traking of things which seems logical. On the \
other hand, it creates only one UndoCommand (with the startMacro/stopMacro mechanism) \
were, from a user perspective, one edit action has been done (see example of carriage \
return). This is how you arrive at situations were the QTextDocument created 3 \
undoCommands and the TextShape only one: when pressing undo once, the call for \
QTextDocumment::undo will actually only undo a part of the whole actions.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">I have several ideas to solve \
the out-of-sync problem. However, before going too far in one direction, it would be \
better to know whether Qt is faulty or if its behavior was planned like that.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">Pierre</p></body></html>



_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic