Re: KWPageManager::pages question
On Saturday, December 05, 2015 02:24:43 PM Friedrich W. H. Kossebau wrote: > Hi Thorsten, > > Am Samstag, 5. Dezember 2015, 06:24:11 schrieb Thorsten Zachmann: > [snip] > > > Is it ok to add that or is the comment wrong and I could use the sort > > inside cstester. I don't know if any other code depends on the pages > > being sorted > Looks like some code expects the pages ordered by page order, e.g. > KWDocument::updatePagesForStyle(const KWPageStyle ) > KWOdfWriter::save(KoOdfWriteStore , KoEmbeddedDocumentSaver > ) > KWViewModeNormal::updatePageCache() > > So fixing the implementation of KWPageManager::pages(...) to return the list > sorted might be needed in general, yes. > Not sure though by what is should be sorted. pageNumber or pageId. The > latter is what is used by KWPage compare operators currently. > > Would need more investigation... but good catch for now at least. At least for what it is used in cstester that is fine. I will commit it as that for now. If we find we need something different we can still update that. > And I would prefer a qSort over std::sort, at least for now for consistency > in the code. qSort is deprecated in Qt5 and I think we should try to move away from deprecated stuff. > Given newer C++ standards etc. it might be time to reconsider the Calligra > coding standards perhaps, e.g. Krita have a nice shot at it with > https://community.kde.org/Krita/C%2B%2B11 Thorrsten ___ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
Re: KWPageManager::pages question
Hi Thorsten, Am Samstag, 5. Dezember 2015, 06:24:11 schrieb Thorsten Zachmann: [snip] > Is it ok to add that or is the comment wrong and I could use the sort inside > cstester. I don't know if any other code depends on the pages being sorted Looks like some code expects the pages ordered by page order, e.g. KWDocument::updatePagesForStyle(const KWPageStyle ) KWOdfWriter::save(KoOdfWriteStore , KoEmbeddedDocumentSaver ) KWViewModeNormal::updatePageCache() So fixing the implementation of KWPageManager::pages(...) to return the list sorted might be needed in general, yes. Not sure though by what is should be sorted. pageNumber or pageId. The latter is what is used by KWPage compare operators currently. Would need more investigation... but good catch for now at least. And I would prefer a qSort over std::sort, at least for now for consistency in the code. Given newer C++ standards etc. it might be time to reconsider the Calligra coding standards perhaps, e.g. Krita have a nice shot at it with https://community.kde.org/Krita/C%2B%2B11 Cheers Friedrich ___ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
KWPageManager::pages question
Hello,
when working on getting cstester ported I noticed that the pages are in a
random oder all the time. Looking at the code I see that the pages are stored
in q QHash and it seems that at least in Qt5 the order is not always the same
in every run.
In the header it is says that the pages are orderd.
/**
* Return an ordered list of all pages.
* @param pageStyle if non empty return only the pages that follow the
page style.
*/
QList pages(const QString = QString()) const;
However when looking at the implementation you can clearly see that this is
not the case.
QList KWPageManager::pages(const QString ) const
{
QList answer;
const bool checkForStyle = !pageStyle.isEmpty();
QHash::ConstIterator it = d-
>pages.constBegin();
QHash::ConstIterator end = d-
>pages.constEnd();
for(; it != end; ++it) {
if (checkForStyle && it.value().style.name() != pageStyle)
continue;
answer << KWPage(d, it.key());
}
return answer;
}
Adding a
std::sort(answer.begin(), answer.end()) ;
before returning fixes the problem of the pages not being sorted.
Is it ok to add that or is the comment wrong and I could use the sort inside
cstester. I don't know if any other code depends on the pages being sorted .
Have a nice day,
Thorsten
___
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
