Re: Review Request: Default - empty char format is used for inline (as-char) shapes.

> On 2010-11-18 16:49:17, Casper Boemann wrote:
> > Obviously the argument of not using bool is valid, but in this case using a charformat, would mean many pleases where calling with the charformat of the cursor would be needed.
> > A bool that defaults to false means much fewer places that needs code update.
> > I'm just saying that coding for the future pays an impact in the present.
> >
> 
> Thomas Zander wrote:
>     >  using a charformat, would mean many pleases where calling with the charformat of the cursor would be needed.
>     
>     Can you explain why that is so?
>     Both a bool can have a default ( = false) and a QTextCharFormat can also have a default  (see QTextCursor API, insertBlock for instance). So there is not really any difference as far as I can see. Am I missing something?
>     
>     Unless I'm missing the point of this patch, the default empty charFormat is what you want to use in most usecases. The latest revision of the patch is missing usecases right now that could easily be fixed but are not with the approach Pavol choose. So I'm wondering why that he didn't follow my advice and went another way which introduces more bugs.
>     For instance if a user inserts an image in his current document then the exact same problem will appear and there may be underlines and all that stuff on his image.
>     
>     I suggested a solution that solves this in a previous thread that would solve both issues in one go. Is there any reason to not use that approach?

Well I suggested that he examine the bool road, because I was under the impression that most cases would require the textformat of the current cursor. Even though QTextCharFormat can have a default value that wouldn't help much if most cases would need a specific value. A specific value that could easily be computed inside the method. That is why I suggest to investigate the bool.

However if indeed only a few usecases need a non-blank QTextCharFormat then the need for bool is not relevant.

As to why he hasn't fixed all cases I'll let him answer himself, but I assume that this is just a technology demonstrator, getting the feedback he is getting now. So no need to implement all cases, before he is sure. But obviously all cases should be fixed.


- Casper


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5898/#review8825
-----------------------------------------------------------


On 2010-11-18 11:21:01, Pavol Korinek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/5898/
> -----------------------------------------------------------
> 
> (Updated 2010-11-18 11:21:01)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> insertInlineObject method in KoInlineTextObjectManager class has been changed. Inline object can be inserted using empty - default char format or current cursor char format. Empty - default char format is used for inline (as-char) shapes now.
> 
> 
> Diffs
> -----
> 
>   /trunk/koffice/libs/kotext/KoInlineTextObjectManager.h 1198325 
>   /trunk/koffice/libs/kotext/KoInlineTextObjectManager.cpp 1198325 
>   /trunk/koffice/libs/kotext/opendocument/KoTextLoader.cpp 1198325 
> 
> Diff: http://svn.reviewboard.kde.org/r/5898/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavol
> 
>

_______________________________________________
koffice-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/koffice-devel