Summary: | print: empty fields get sorted semi-random - need multiple sorting rules? | ||
---|---|---|---|
Product: | kab3 | Reporter: | Hans Ecke <hans> |
Component: | general | Assignee: | Tobias Koenig <tokoe> |
Status: | RESOLVED UNMAINTAINED | ||
Severity: | normal | CC: | bernd-kummer, rdieter |
Priority: | NOR | ||
Version: | 3.3 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: |
add default sorting
Naive fix for this bug |
Description
Hans Ecke
2004-10-11 03:36:25 UTC
The relevant file is printingwizard.cpp, line 151-213, function print(). If you just imply the following multiple sorting rules: 1) mStylePage->sortField() 2) Given Name 3) Family Name 4) Formatted Name That is a perfectly reasonable default. I attached a patch to that effect. References: http://developer.kde.org/documentation/library/cvs-api/kabc/html/classKABC_1_1AddresseeList.html http://korganizer.kde.org/dox/kaddressbook/html/printingwizard_8cpp-source.html Created attachment 7834 [details]
add default sorting
The above solution works, which is good. However, it is slightly suboptimal: * It does not allow to specify multiple sorting rules but relies on what the developer thinks is a good default * It is slow. "KABC::AddresseeList list" is sorted 3 times by addresseelist.cpp:sortByTrait() which is a bubblesort. That should be refactored to a generic (sort <-> comparison object), where the comparison object takes a list of sort fields. Then it would just be one call to that generic sort. References: http://webcvs.kde.org/cgi-bin/cvsweb.cgi/kdepim/kaddressbook/printing/printingwizard.cpp?rev=1.16;content-type=text%2Fplain Created attachment 7839 [details]
Naive fix for this bug
Okay, this patch solves the problem for me. I recompiled kdelibs and kdepim and
now printing imposes a default sort order that resolves "ties" between contact
where the sort field is not available.
There are 2 parts to this:
* printingwizard.cpp: impose default sorting order
list.sortBy() is a bubblesort, so it is stable. Subsequent sortBy() will not
destroy prior order.
* addresseelist.cpp:sortByField(): change the qHeapSort to a qBubbleSort. The
heap sort is not stable, so it would destroy the default ordering.
The bad thing about this is that in printingwizard.cpp:print(), we do 4 bubble
sorts of the whole address list. If the sorting function could understand to
sort by multiple fields we could just use one single heap sort. On the other
hand, this code path is not taken often and only interactively. So maybe it
doesn't matter much.
I'll shut up now :-)
Hi Tobias, Could you please look at this? This bug can be closed with a minimum of work on your part. There is a patch. This patch works fine on all the computers I admin here at the university. Rex Dieter has incorporated it into his kde-redhat distribution and it seems to work just fine for everybody. This is a huge testing base. All you'd need to do is to apply the patch and close this bug. I understand you have a lot of work to do and I really appreciate your work on kaddressbook, but this would be really really easy to fix.... Thanks Hans *** Bug 92896 has been marked as a duplicate of this bug. *** CVS commit by tokoe: Use givenName, familyName, formattedName as additional sort criterions when the primary sort field is equal/empty. BUG:91096 A printsortmode.cpp 1.1 [GPL (v2+) (+Qt exception)] A printsortmode.h 1.1 [GPL (v2+) (+Qt exception)] M +2 -1 Makefile.am 1.19 M +8 -0 printingwizard.cpp 1.17 --- kdepim/kaddressbook/printing/Makefile.am #1.18:1.19 @@ -16,5 +16,6 @@ rbs_appearance.ui \ selectionpage.cpp \ - stylepage.cpp + stylepage.cpp \ + printsortmode.cpp libprinter_la_COMPILE_FIRST = $(top_builddir)/kaddressbook/common/kabprefs_base.h --- kdepim/kaddressbook/printing/printingwizard.cpp #1.16:1.17 @@ -48,4 +48,5 @@ #include "printprogress.h" #include "printstyle.h" +#include "printsortmode.h" #include "printingwizard.h" @@ -201,5 +202,12 @@ void PrintingWizard::print() list.setReverseSorting( !mStylePage->sortAscending() ); + +#if KDE_IS_VERSION(3,3,91) + qDebug("printsortmode"); + PrintSortMode sortMode( mStylePage->sortField() ); + list.sortByMode( &sortMode ); +#else list.sortByField( mStylePage->sortField() ); +#endif } The development of the old KAddressBook will be discontinued for KDE 4.4. Since the new application has the same name, but a completly new code base we close all bug reports against the old version and ask the submitters to resend there reports against the new product. |