[bt-devel] Config system

[Eeli Kaikkonen]

Quoting Gary Holmlund <gary.holmlund at gmail.com>:

> I think I would favor multiple access functions. The .toString function

>
> 1. If the QVariant value was an int, but you request a string you will
> get a string, not an error.
>
>
> 2. Inside a typed access function you can test the type of the QVariant
> and Assert that it is of the right type.
>
> 3. Did it return an empty string because the string value was empty or
> because it was an invalid type to convert to a string?
>
>
> 4. With a typed access function you will you will probably do less
> checking of the value after the access function is called. This will
> make the code somewhat cleaner where the access function is used.
>

The setter is more important for type-safety and it does the checks  
already. It is true that toXXX() can return a value even if it doesn't  
match the original, but for real coding, is a check really needed? If  
someone uses a config item, he knows what it is, right? If it works,  
fine. If not, he was careless. The setter takes care of storing only  
valid types. Many times the setter and getter are used close to each  
other (e.g. init() and save() functions) and the code is written at  
the same time. The setter catches the type mismatch.

However, there is a much more serious flaw in my BtConfig. It doesn't  
handle int lists or any other complex types. It means that we have to  
either make them known to QVariant or write specialized getters and  
possibly setters anyways. Therefore I will not do this at least at the  
moment.


> I am not sure why it makes the Action porting easier. The actions are
> created in the various window classes using values from the CResMgr.
> The value of the new config system for Actions comes in when we are
> ready to values other than the default value. That is when we would
> have a ShortCut editor, not now. Here are a couple of lines that
> illustrate getting a default value for an Action from CResMgr:
>    kaction = new KAction(tr("Previous chapter"), a);
>
> kaction->setShortcut(CResMgr::displaywindows::bibleWindow::previousChapter::accel);
>

The config system should have been changed when KDE3 actions were  
ported to KDE4. The current system forced me to write every line  
manually instead of writing a simple action creator function with few  
arguments. The code is messy and long now, with the new config system  
it would be about one line per action. If there is now a KAction(...)  
ctor call somewhere we have to change it to QAction(...) which is  
annoying because they are different, so we have to move every argument  
and probably add QAction member calls. That was what I had to do with  
KDE3->4 port, and it wasn't fun. It could be avoided if the incomplete  
CResMgr values could be used as arguments, but they cannot (see the  
wiki page for this).

Please note that this is based on my personal feelings and my former  
experiences with the code. Fortunately there is much less left to do  
now and the rest may be easier than I remember.

> Since we are not using the KDE ShortCut editor, the only function of
> the KActionCollection class is in providing a container for Actions and
> a method of looking them up by a name. Even this functionality is not
> needed until we write the ShortCut editor.  So, KActionCollection is
> doing NOTHING for us right now. It can be deleted without any loss of
> current functionality.
>

> The value of replacing KActionCollection with a BtActionCollection
> (which would be a container with named access to actions) only becomes
> important when we start writing a ShortCut editor.  The BtConfig system
> may provide  an alternate  way to  access  actions  by name, I am not
> sure.

They don't have to be accessed by name. The configurator needs: icon,  
tr()'ed string, shortcut. And a way to store the shortcut with the  
correct key. The most simple way would be something like this:

The configurator gets maps of key string/tr() string from windows:
     "biblereadwindow/actions/nextverse" "Next verse"
     "biblereadwindow/actions/prevverse" "Previous verse"
Now it can loop through the map and fetch the icon and the shortcut  
because the keys are standardized:
     BtConfig::get("biblereadwindow/actions/nextverse" + "icon") [pseudocode]
The tr()'ed UI strings are not stored in config, therefore they have  
to be given separately.

Of course it's also possible to create some simple struct with the  
necessary data.

> I think the right order for me to proceed is:
>
> 1. Convert KActions to QActions
> 2. Delete the KActionCollection and not replace it.
> 3. Get back to replacing KApplication with QApplication.
> 4. Do the About dialog.
>
> I don't think these tasks will be very long.
>

I guess you are right. There are difficulties with standard actions  
and KAction subclasses. For example KToolBarPopupAction needs to be  
replaced with QWidgetAction or something else. It takes a little bit  
more time than just plain KAction, but should still be straightforward.

I'm eager to get BT compile under Windows, so we can leave even the  
About dialog later.

--Eeli Kaikkonen