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

List:       koffice-devel
Subject:    Re: Review Request: Import ToC without modifying it
From:       Jean-Nicolas Artaud <jeannicolasartaud () gmail ! com>
Date:       2010-05-28 9:33:49
Message-ID: AANLkTilrUSYy3pysUhzm0U1UCarN3UlZdhzfjpwlpZYc () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


2010/5/27 Thomas Zander <zander@kde.org>

> You replied on the mailinglist and not on the review request, please in
> future
> keep the conversation in one place by replying to the review request.
>

Yes, sorry, I though that was the way we proceed when we begin to discuss
some
points, and not directly the patch reviewed, my bad.


>
>
>
> On Wednesday 26. May 2010 09.57.07 you wrote:
> > > So you are saying the ToC should not be updated when its loaded?
> > Yes, I am.
> >
> > > I disagree if thats what you are saying, the ToC has to be updated
> because
> > > page numbers may not be correct. In fact we need to also make sure the
> > > ToC is regenerated before saving and printing to make totally sure the
> > > page numbers are correct.
> >
> > I really do agree for the pages but not only page numbers are updated,
> but
> > all the lines of the ToC are regenerated, that's what is bad.
>
> Thats expected. I see no reason why that would be bad.
>

It is bad, because kword is not adapted at all with the outline level, so,
it loads empty tocs...


>
> > So, if the outline number is not correctly set (yes, kword is totally
> wrong
> > in this way) we load empty ToC instead of the good one.
>
> I know, there are many many bugs in ToC generation right now. The ToC is
> generated for an A4 width and won't work on any other width, for instance.
> We
> don't have any unit tests for this feature either.
> All these bugs need to be fixed before we can ship this new feature in an
> actual
> KOffice release.
>

It does make sense.


> > At least my patch fixed this !
>
> I plan to disable ToC Generation for the upcoming release if it doesn't do
> the
> basic things correctly. But only at the time of release. So in trunk we can
> see
> stuff going wrong, and we can fix it.
>

Go ahead.


> Working around it with your patch will make basic functionality even worse.
> If
> a user adds a new chapter it should definitely be shown in the ToC. Which
> becomes impossible with your patch.
>
> I think its important to aim for the correct solution and release the
> feature
> only when its done. So your suggestion to disable this will end up being
> done
> for 2.3 by me as I agree with you that the current behavior is bad.
> Ideally we can fix all the issues before that date so I don't have to
> disable
> it :)
>
> --
> Thomas Zander
> _______________________________________________
> koffice-devel mailing list
> koffice-devel@kde.org
> https://mail.kde.org/mailman/listinfo/koffice-devel
>

[Attachment #5 (text/html)]

<br><br><div class="gmail_quote">2010/5/27 Thomas Zander <span dir="ltr">&lt;<a \
href="mailto:zander@kde.org">zander@kde.org</a>&gt;</span><br><blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt \
0pt 0.8ex; padding-left: 1ex;"> You replied on the mailinglist and not on the review \
request, please in future<br> keep the conversation in one place by replying to the \
review request.<br></blockquote><div><br>Yes, sorry, I though that was the way we \
proceed when we begin to discuss some<br>points, and not directly the patch reviewed, \
my bad.<br>  </div><blockquote class="gmail_quote" style="border-left: 1px solid \
rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div \
class="im"><br> <br>
<br>
On Wednesday 26. May 2010 09.57.07 you wrote:<br>
&gt; &gt; So you are saying the ToC should not be updated when its loaded?<br>
&gt; Yes, I am.<br>
&gt;<br>
&gt; &gt; I disagree if thats what you are saying, the ToC has to be updated \
because<br> &gt; &gt; page numbers may not be correct. In fact we need to also make \
sure the<br> &gt; &gt; ToC is regenerated before saving and printing to make totally \
sure the<br> &gt; &gt; page numbers are correct.<br>
&gt;<br>
&gt; I really do agree for the pages but not only page numbers are updated, but<br>
&gt; all the lines of the ToC are regenerated, that&#39;s what is bad.<br>
<br>
</div>Thats expected. I see no reason why that would be bad.<br></blockquote><div> \
</div><div>It is bad, because kword is not adapted at all with the outline level, so, \
it loads empty tocs...<br> <br></div><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; \
padding-left: 1ex;">

<div class="im"><br>
&gt; So, if the outline number is not correctly set (yes, kword is totally wrong<br>
&gt; in this way) we load empty ToC instead of the good one.<br>
<br>
</div>I know, there are many many bugs in ToC generation right now. The ToC is<br>
generated for an A4 width and won&#39;t work on any other width, for instance. We<br>
don&#39;t have any unit tests for this feature either.<br>
All these bugs need to be fixed before we can ship this new feature in an actual<br>
KOffice release.<br></blockquote><div> </div><div>It does make \
sense.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid \
rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div \
class="im"><br> &gt; At least my patch fixed this !<br>
<br>
</div>I plan to disable ToC Generation for the upcoming release if it doesn&#39;t do \
the<br> basic things correctly. But only at the time of release. So in trunk we can \
see<br> stuff going wrong, and we can fix it.<br></blockquote><div><br> Go ahead.<br> \
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, \
204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> Working around it with your \
patch will make basic functionality even worse. If<br> a user adds a new chapter it \
should definitely be shown in the ToC. Which<br> becomes impossible with your \
patch.<br> <br>
I think its important to aim for the correct solution and release the feature<br>
only when its done. So your suggestion to disable this will end up being done<br>
for 2.3 by me as I agree with you that the current behavior is bad.<br>
Ideally we can fix all the issues before that date so I don&#39;t have to disable<br>
it :)<br>
<font color="#888888"><br>
--<br>
Thomas Zander<br>
_______________________________________________<br>
koffice-devel mailing list<br>
<a href="mailto:koffice-devel@kde.org">koffice-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/koffice-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/koffice-devel</a><br> \
</font></blockquote></div><br>



_______________________________________________
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