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

List:       koffice-devel
Subject:    Re: a tool for editing paragraph spacing
From:       Thorsten Zachmann <t.zachmann () zagge ! de>
Date:       2008-02-28 21:25:55
Message-ID: 200802282225.55321.t.zachmann () zagge ! de
[Download RAW message or body]

Hello Florian,
> Hi KOffice developers,
>
> I've been working on a KoTool for the TextShape which tries to allow
> editing of paragraph spacing and indentation in a more visual and intuitive
> way. Please keep in mind, that right now this is not more than a proof of
> concept. There are lots of bugs and corner cases which need to be taken
> care of before this is ready for some real use. It also needs a few more
> features to be truly useful, such as line spacing, QTextListFormat support
> and QTextTableFormat support.
>

> What do you think about it? 

It is quite easy to use and looks quite good.

> Any suggestions or ideas? 

How about make it possible to move the lines freely by pressing shift to 
disable the snapping to the predefined steps.

> If this is considered  
> useful for koffice I'd be thankful for a code review. I'm not too sure I
> used the koffice libraries properly...

looks quite good. Here some remarks

+++ plugins/textshape/ParagraphSpacingToolFactory.cpp

> + * Copyright (C) 2006 Florian Merz <florianmerz@web.de>

Did you really do this 2 years ago :-)

+++ plugins/textshape/HRuler.h

> +#ifndef HARROWCONTROL_H

the includeguard should match the filename. This i also true for some other 
classes.

+++ plugins/textshape/HRuler.cpp

> +#include <stdio.h>

looks like this is no longer needed. The same is true for the one in 
VRuler.cpp.

+++ plugins/textshape/Ruler.h   (Revision 0)

> +    void setBaseline(QPointF position, qreal lineLength);

classes like QPointF should be passed as const reference.

+++ plugins/textshape/ParagraphSpacingTool.h

> +#include "TextShape.h"

> +#include <KoParagraphStyle.h>
> +#include <KoCanvasBase.h>

this should be moved to ParagraphSpacingTool.cpp and a forward declaration 
used.

> +    Ruler *m_firstIndentRuler,
> +          *m_followingIndentRuler,
> +          *m_rightMarginRuler,
> +          *m_topMarginRuler,
> +          *m_bottomMarginRuler;

Is there a special reason for creating them on the heap and not on the stack?

When testing in kpresenter I found a problem with displaying the rulers when 
the space in the shape is no longer big enough to hold all the text. See 
http://www.zagge.de/files/paragraph_spacing.png .

Looks very nice.

Thorsten




_______________________________________________
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]