Code review comment for lp://qastaging/~christian-w/lightdm/qt-binding-keyboard-layouts

37 + * Copyright (C) 2010-2011 David Edmundson.
38 + * Author: David Edmundson <email address hidden>

Either I was very drunk when writing this and forgot, or you've clearly copy and pasted this :)
Remember to set 2014 too.

------

+void LayoutsModel::setCurrentLayout(QVariant layoutData)

const & layoutData. Save yourself a copy.

--

I would register LightDMLayout as a metatype
Q_DECLARE_METATYPE(LightDMLayout)

and then you shouldn't need to cast to void*.

Right now if I did
myLayoutModel.layout = someRandomQObject from inside the QML it will crash.

QVariant will be able to cast it to void* and static_cast<> almost always succeeds.
We don't want to let the greeters be able to cause a crash.

--

As for the API. I know what you mean, it's a bit sucky but I would agree with you that this is one of the least terrible ways to do it.

I'd change it to a property. Otherwise you need to have a component.onCompleted which is messy; you have the data when you load, you may as well bind to it initially.

The other approach I've seen is to have a property with the current index. This has the disadvantage of getting confusing if the model becomes dynamicly changing; but has the awesome advantage that you can select the right index in a combo box etc without having to have an awkward loop in javascript.

--
LayoutsModel::data

FYI if you want to pass against QtModelTest (http://qt-project.org/wiki/Model_Test) you'll need to check bounds here i.e row > 0, row < rowCount(), column == 0 etc and return a QVariant otherwise.
I doubt I do it anywhere else, so I don't really care about this one.