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

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


Except for these small remarks the patch looks really good. Can you
have a look at these small issues? The patch also contains a nice
clean-up in the dialog code that will be a good start for the new ui.

Thanks,
Markus