Re: koffice/libs/main

On Wed September 16 2009, Boudewijn Rempt wrote:

> On Wed, 16 Sep 2009, Thomas Zander wrote:
> > Having a default of 1000 as well as a configurable UI that resets it
> > sounds like a long term solution to me. Its silly to have no limit at
> > all, unless the user actually configures that.
>
> At least get the limit from the configuration file instead of hardcoding
>  it.
>
> > Setting the limit at the 'new' of the stack means that a UI option
> > actually gets possible to set, anything else raises the question which
> > one sets the limit first.
>
> It's better to use a config file to get the setting. I won't quibble about
>  the default if the entry isn't present, but it would be cleaner than using
>  a hard coded number.
>
> (And I still think your real bug is that you add undo actions during the
>  loading of a document.)
>
> Boudewijn
>

Can the bug (adding undo commands during loading) please be fixed. This causes
a lot of undo commands to be generated during copy and paste of pages which
contain text shapes and when importing documents into a document that contains
text shape into a kopage app. When Pierre was talking to me about the
KoTextEditor changes and we agreed that there won't be any commands generated
during loading.

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

On Friday 18. September 2009 20.42.22 Thorsten Zachmann wrote:
> Can the bug (adding undo commands during loading) please be fixe

I think I fixed it in svn rev r1026312, committed that just now.

--
Thomas Zander

Qt Developer Days 2009 | Early Bird Pricing - Save €200/$200!
Munich, Germany: October 12th - 14th
San Francisco, California: November 2nd - 4th
Register now! http://qt.nokia.com/qtdevdays2009



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

On Mon September 21 2009, Thomas Zander wrote:
> On Friday 18. September 2009 20.42.22 Thorsten Zachmann wrote:
> > Can the bug (adding undo commands during loading) please be fixe
>
> I think I fixed it in svn rev r1026312, committed that just now.
>
The problem in kpresenter is fixed. However it is no longer possible to undo a
paste of a text.

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

On Wednesday 23. September 2009 14.40.57 Thorsten Zachmann wrote:
> On Mon September 21 2009, Thomas Zander wrote:
> > On Friday 18. September 2009 20.42.22 Thorsten Zachmann wrote:
> > > Can the bug (adding undo commands during loading) please be fixe
> >
> > I think I fixed it in svn rev r1026312, committed that just now.
>
> The problem in kpresenter is fixed. However it is no longer possible to
> undo a paste of a text.

Could you give a bit more detail? It works fine here.
--
Thomas Zander

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

On Wed September 23 2009, Thomas Zander wrote:

> On Wednesday 23. September 2009 14.40.57 Thorsten Zachmann wrote:
> > On Mon September 21 2009, Thomas Zander wrote:
> > > On Friday 18. September 2009 20.42.22 Thorsten Zachmann wrote:
> > > > Can the bug (adding undo commands during loading) please be fixe
> > >
> > > I think I fixed it in svn rev r1026312, committed that just now.
> >
> > The problem in kpresenter is fixed. However it is no longer possible to
> > undo a paste of a text.
>
> Could you give a bit more detail? It works fine here.
>

Looks like I had some local changes that messed up everything. Sorry about
that.

o Copy and paste of text is undoable.

o If you copy and paste a page in kpresenter or if you import a slideshow into
a presentation a lot of undo commands are still added. The commands are of
type UndoTextCommand.

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

Hello Thomas,

On Thu September 24 2009, Thorsten Zachmann wrote:

> On Wed September 23 2009, Thomas Zander wrote:
> > On Wednesday 23. September 2009 14.40.57 Thorsten Zachmann wrote:
> > > On Mon September 21 2009, Thomas Zander wrote:
> > > > On Friday 18. September 2009 20.42.22 Thorsten Zachmann wrote:
> > > > > Can the bug (adding undo commands during loading) please be fixe
> > > >
> > > > I think I fixed it in svn rev r1026312, committed that just now.
> o Copy and paste of text is undoable.
>
> o If you copy and paste a page in kpresenter or if you import a slideshow
>  into a presentation a lot of undo commands are still added. The commands
>  are of type UndoTextCommand.

The problem of adding commands during loading is not fixed. Can you please
have a look as this breaks undo for import of slideshows in kpresenter.

Thanks,

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

On Friday 23. October 2009 20.46.35 you wrote:
> Hello Thomas,
>
> On Thu September 24 2009, Thorsten Zachmann wrote:
> > On Wed September 23 2009, Thomas Zander wrote:
> > > On Wednesday 23. September 2009 14.40.57 Thorsten Zachmann
wrote:
> > > > On Mon September 21 2009, Thomas Zander wrote:
> > > > > On Friday 18. September 2009 20.42.22 Thorsten Zachmann
wrote:

> > > > > > Can the bug (adding undo commands during loading) please be
> > > > > > fixe
> > > > >
> > > > > I think I fixed it in svn rev r1026312, committed that just now.
> >
> > o Copy and paste of text is undoable.
> >
> > o If you copy and paste a page in kpresenter or if you import a
> > slideshow into a presentation a lot of undo commands are still added.
> > The commands are of type UndoTextCommand.
>
> The problem of adding commands during loading is not fixed. Can you
> please have a look as this breaks undo for import of slideshows in
> kpresenter.

I likely won't have time for this, Thorsten, sorry.
I recall I suggested you fix kpresenter to create a parent undocommand to
wrap the action into one undo command. Did that not work out?
Why are you mailing me directly with the request to fix a kpresenter bug?
--
Thomas Zander


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

On Sat October 24 2009, Thomas Zander wrote:
> On Friday 23. October 2009 20.46.35 you wrote:
> > Hello Thomas,
> >
> > On Thu September 24 2009, Thorsten Zachmann wrote:
> > > On Wed September 23 2009, Thomas Zander wrote:

>
> wrote:

>
> wrote:

> > >
> > > o Copy and paste of text is undoable.
> > >
> > > o If you copy and paste a page in kpresenter or if you import a
> > > slideshow into a presentation a lot of undo commands are still added.
> > > The commands are of type UndoTextCommand.
> >
> > The problem of adding commands during loading is not fixed. Can you
> > please have a look as this breaks undo for import of slideshows in
> > kpresenter.
>
> I likely won't have time for this, Thorsten, sorry.
> I recall I suggested you fix kpresenter to create a parent undocommand to
> wrap the action into one undo command. Did that not work out?

I'm not sure I fully understand what you mean. The problem with a lot of
commands can be fixed by using beginMacro/endMacro. Problem with that is that
it is not possible to change the text of the command afterwards. But only then
it is know if one or more pages are added.

> Why are you mailing me directly with the request to fix a kpresenter bug?

It is not a kpresenter bug but a bug of the text shape that it creates
commands during loading. This bug was introduced by your changes with the
KoTextEditor. So I pointed you to the problems that it creates and had hopes
you would fix the problems of your changes. However my hopes have not come
true.

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

On Sunday 25. October 2009 13.25.42 Thorsten Zachmann wrote:
> > I recall I suggested you fix kpresenter to create a parent undocommand
> > to wrap the action into one undo command. Did that not work out?
>
> I'm not sure I fully understand what you mean. The problem with a lot of
> commands can be fixed by using beginMacro/endMacro. Problem with that
is
> that it is not possible to change the text of the command afterwards. But
> only then it is know if one or more pages are added.

Maybe you can use a generic text?

I'm not following how this factors in with text shape loading at all, to be
honest. Maybe I'm missing your point... :(
Even if text loading doesn't create any undo commands you will still have
one undo command per inserted shape unless you use
beginMacro/endMacro.

And if you use those two then any bug in the textEditor is not that much of
an issue. Would be nice to fix it, but its really a completely different bug.

Did you try that solution?  It has the advantage of affecting only
kpresenter :)
--
Thomas Zander


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

On Sun October 25 2009, Thomas Zander wrote:

> On Sunday 25. October 2009 13.25.42 Thorsten Zachmann wrote:
> > > I recall I suggested you fix kpresenter to create a parent undocommand
> > > to wrap the action into one undo command. Did that not work out?
> >
> > I'm not sure I fully understand what you mean. The problem with a lot of
> > commands can be fixed by using beginMacro/endMacro. Problem with that
>
> is
>
> > that it is not possible to change the text of the command afterwards. But
> > only then it is know if one or more pages are added.
>
> Maybe you can use a generic text?
>
> I'm not following how this factors in with text shape loading at all, to be
> honest. Maybe I'm missing your point... :(
> Even if text loading doesn't create any undo commands you will still have
> one undo command per inserted shape unless you use
> beginMacro/endMacro.

The only commands that are generated during loading are commands of the
KoTextEditor. Nothing else.

> And if you use those two then any bug in the textEditor is not that much of
> an issue. Would be nice to fix it, but its really a completely different
>  bug.

Patch is attached. Please review. Additionally it allows use to remove some
workarounds for this problem.

Thorsten

[no_commands_on_load.diff]

---

Index: libs/kotext/KoTextEditor_p.h
===================================================================
--- libs/kotext/KoTextEditor_p.h (revision 1040051)
+++ libs/kotext/KoTextEditor_p.h (working copy)
@@ -64,6 +64,7 @@ public:
     QString commandTitle;
     KoText::Direction direction;
     bool isBidiDocument;
+    bool addCommand;
 
     State editorState;
 
Index: libs/kotext/KoTextEditor.h
===================================================================
--- libs/kotext/KoTextEditor.h (revision 1040051)
+++ libs/kotext/KoTextEditor.h (working copy)
@@ -72,6 +72,8 @@ QTextCursor ( const QTextCursor & cursor
 
     bool operator>=(const QTextCursor &other) const;
 
+    void setAddCommand(bool add);
+
 public slots:
     ///This should be used only as read-only cursor or within a QUndoCommand sub-class which will be added to the textEditor with addCommand. For examples of proper implementation of such undoCommands, see the TextShape commands.
     QTextCursor* cursor();
Index: libs/kotext/KoTextEditor.cpp
===================================================================
--- libs/kotext/KoTextEditor.cpp (revision 1040051)
+++ libs/kotext/KoTextEditor.cpp (working copy)
@@ -87,7 +87,8 @@ KoTextEditor::Private::Private(KoTextEdi
     : q(qq),
     document (document),
     headCommand(0),
-    isBidiDocument(false)
+    isBidiDocument(false),
+    addCommand(true)
 {
     caret = QTextCursor(document);
     editorState = NoOp;
@@ -120,6 +121,10 @@ void KoTextEditor::Private::documentComm
 
         QPointer<QTextDocument> m_document;
     };
+    if (!addCommand) {
+        return;
+    }
+
     //kDebug() << "editor state: " << editorState << " headcommand: " << headCommand;
     if (!headCommand || editorState == NoOp) {
         headCommand = new QUndoCommand(commandTitle);
@@ -307,6 +312,11 @@ void KoTextEditor::updateDefaultTextDire
     d->direction = direction;
 }
 
+void KoTextEditor::setAddCommand(bool add)
+{
+    d->addCommand = add;
+}
+
 QTextCursor* KoTextEditor::cursor()
 {
     return &(d->caret);
Index: plugins/textshape/TextShape.cpp
===================================================================
--- plugins/textshape/TextShape.cpp (revision 1040051)
+++ plugins/textshape/TextShape.cpp (working copy)
@@ -329,6 +329,7 @@ void TextShape::saveOdf(KoShapeSavingCon
 
 bool TextShape::loadOdf(const KoXmlElement &element, KoShapeLoadingContext &context)
 {
+    KoTextDocument(m_textShapeData->document()).textEditor()->setAddCommand(false);
     loadOdfAttributes(element, context, OdfAllAttributes);
 
     // load the (text) style of the frame
@@ -361,7 +362,9 @@ bool TextShape::loadOdf(const KoXmlEleme
         paragraphStyle.applyStyle(block, false);
     }
 
-    return loadOdfFrame(element, context);
+    bool retval = loadOdfFrame(element, context);
+    KoTextDocument(m_textShapeData->document()).textEditor()->setAddCommand(true);
+    return retval;
 }
 
 bool TextShape::loadOdfFrameElement(const KoXmlElement &element, KoShapeLoadingContext &context)
Index: kpresenter/part/KPrPlaceholderTextStrategy.cpp
===================================================================
--- kpresenter/part/KPrPlaceholderTextStrategy.cpp (revision 1040051)
+++ kpresenter/part/KPrPlaceholderTextStrategy.cpp (working copy)
@@ -37,6 +37,7 @@
 #include <KoShapeSavingContext.h>
 #include <KoTextShapeData.h>
 #include <KoTextDocument.h>
+#include <KoTextEditor.h>
 #include <KoTextDocumentLayout.h>
 #include <KoStyleManager.h>
 #include <KoXmlReader.h>
@@ -142,7 +143,9 @@ bool KPrPlaceholderTextStrategy::loadOdf
         Q_ASSERT( factory );
         m_textShape = factory->createDefaultShapeAndInit( context.dataCenterMap() );
 
-        KoTextShapeData * shapeData = qobject_cast<KoTextShapeData*>(  m_textShape->userData() );
+        KoTextShapeData * shapeData = qobject_cast<KoTextShapeData*>( m_textShape->userData() );
+        KoTextDocument( shapeData->document() ).textEditor()->setAddCommand( false );
+
         QTextDocument * document = shapeData->document();
         QTextCursor cursor( document );
         QTextBlock block = cursor.block();
@@ -150,6 +153,7 @@ bool KPrPlaceholderTextStrategy::loadOdf
         cursor.insertText( text() );
         shapeData->foul();
         m_paragraphStyle = paragraphStyle.clone();
+        KoTextDocument( shapeData->document() ).textEditor()->setAddCommand( true );
     }
 
     styleStack.restore();

---

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

On Monday 26. October 2009 05.29.42 Thorsten Zachmann wrote:
> > I'm not following how this factors in with text shape loading at all,
> > to be honest. Maybe I'm missing your point... :(
> > Even if text loading doesn't create any undo commands you will still
> > have one undo command per inserted shape unless you use
> > beginMacro/endMacro.
>
> The only commands that are generated during loading are commands of the
> KoTextEditor. Nothing else.

Wait, that doesn't make any sense.
If thats the case you can not undo the inserting of the presentation at all.

Ok, I just tried this and there is no bug. It works exactly as expected.

How do I reproduce this bug? Is it really a bug?
--
Thomas Zander
_______________________________________________
koffice-devel mailing list
koffice-devel@...
https://mail.kde.org/mailman/listinfo/koffice-devel