[PATCH] [WIP] fdo#45747 - [EasyHack] remove the limitation to 3 sort entries in calc

[Albert Thuswaldner]

Hi Markus,
Thanks for your review.

On Thu, Mar 22, 2012 at 01:14, Markus Mohrhard
<markus.mohrhard at googlemail.com> wrote:
> Hey Albert,
>
>> I will check this patch tomorrow. A quick look through the patch did
>> not show any serious problems only some small nit-picks that I will
>> change while reviewing.
>>
>
> Ok. So as promised I reviewed the patch now. Sadly there are some more
> problems than I thought after my first quick look:
>
> - please use Sc instead of SC for type prefix names
> - Can you recheck ScSortParam constructos that already fill
> maKeyState? std::vector[] does not increase the vector size so you are
> only allowed to access existing entries. I think we should use
> push_back there.
> - It seems that you have still some whitespace changes in
> ScTabPageSortFields::Reset
> - personally I'm not a huge fan of hungarian notation but since we are
> using it it would be great if you could use aNewSortParam instead of
> theNewSortParam
> - personally I prefer static_cast in c++ over the old c-cast but I
> have no strong opinion there
> - ScSortDescriptor::FillSortParam looks like there might happen some
> index out of bounds access

So I have updated the patch with respect to your comments above and
corrected all the issues except for the last one, which I honestly can
not find. There are several ScSortDescriptor::FillSortParam in
different files which are touch by my patch. So could help me pointing
out the line(s) which you are concern about?

Regarding the remaining UI bits I have started to look at this and
think I understand how to implement it. The scroll handler is still a
mystery, see if I can figure it out.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fdo-45747-remove-the-limitation-to-3-sort-entries-in.patch
Type: text/x-patch
Size: 63866 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120324/a253b26c/attachment-0001.bin>