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

Hey Albert,

2012/3/20 Albert Thuswaldner <albert.thuswaldner@gmail.com>:
> Hi Markus,
>
> On Thu, Mar 15, 2012 at 04:27, Markus Mohrhard
> <markus.mohrhard@googlemail.com> wrote:
> > Hello Albert,
> >
> > I have a simple idea that shouldn't be too complex to implement. I
> > think we can agree that only the sort entries are the difficult part.
> > I would create a control for an entry an instantiate this one for each
> > new sort entry. I think we do similar things for several other
> > dialogs, e.g. SvTabListBox with simple entries or the document
> > properties dialog with more complex entries.
> Great, Let me know if there is anything that I can do to help.

The idea is to create a normal dialog or reuse the existing dialog.
But instead of hard coding the sort entry positions we need an own
control that just deals with the sort entries.

You can have a look at this idea in ScNamedDlg. ScNameDlg is a dialog
but the table is written in an own control that just deals with range
names and does not need to know anything about the dialog that
contains it. I think a similar approach will work for this here too.
The only difficult part will be the scroll handler. Do you think that
this is something that you can put together?

> > Just two small comments. I think it would be a good idea to group the
> > three entries in ScSortParam into a own struct so that we only need
> > one vector. That will make it easier to keep the entries in sync.
> > And you have a lot of whitespace changes. Can you check your editor
> > why it replaces so many spaces with tabs. It is quite difficult to
> > review it otherwise.
>
> Sorry for the messy patch, I didn't check it properly prior to posting
> it. I have reworked the patch and also split it up into two.
>
> Now the first patch contains only the internal rework to remove the
> limits and converting the data containers from arrays to std:vector.
> I've also grouped the three entries in ScSortParam into a own struct
> as you suggested, which resulted in quite a number of changes.
>
> Once reviewed this patch can be pushed directly to master since it is
> self contained and doesn't change the old
> behavior, me thinks.

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.

Thanks so far for this amazing work. I hope that the UI part does not
sound too scary and am looking forward to a patch for this too. If you
need more help with the UI rework just write a short mail or ping me
in IRC.

Regards,
Markus