Re: [Development] renaming all QWindow properties that have "window" in them
On Mon, Oct 22, 2012 at 10:08:14AM +0200, Samuel Rødal wrote: > > On 19 October 2012 21:56, Oswald Buddenhagen > > wrote: > >> i don't see why this shouldn't apply equally to qml. if it poses a > >> problem in a particular case, it is most likely indicative of a serious > >> API issue. > > What would be indicative of a serious API issue? > property overloading. either it is a different property - then you should have used descriptive names to avoid a clash. or it is actually an attempt to have (virtually) the same property duplicated in the derived class. then you are really in trouble. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
On 10/19/2012 10:18 PM, Shawn Rutledge wrote: > On 19 October 2012 21:56, Oswald Buddenhagen > wrote: >> On Fri, Oct 19, 2012 at 04:46:01PM +, Rutledge Shawn wrote: >>> QWindow has properties like windowTitle, windowIcon, windowModality, >>> windowState and so on, which are named that way to be familiar to users of >>> QWidget. >>> >> correct. http://doc.qt.digia.com/qq/qq13-apis.html#theartofnaming , >> "general naming rules". >> >> i don't see why this shouldn't apply equally to qml. if it poses a >> problem in a particular case, it is most likely indicative of a serious >> API issue. What would be indicative of a serious API issue? I don't really expect QWindow being used in any deep inheritance hierarchies the way QWidget is used. Although, if QWindow is exposed to QML that does impose some constraints. > So pos is bad because it's abbreviated. And I suspect we may have > problems with renaming windowState to state, because in QML that > usually refers to the state-machine state (driving animated > transitions etc.) Should we be inconsistent and leave that one as > windowState then? QCursor, QWidget, QQuickItem, and QGraphicsItem use setPos(), whereas QTextCursor and QTextLayout use setPosition(), so we're not entirely consistent there. As for "state", there's no clash in this case since the Window element won't be a sub-class of QQuickItem. Although it could still lead to confusion, so it might be worth leaving it as "windowState". -- Samuel ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
On sexta-feira, 19 de outubro de 2012 22.18.25, Shawn Rutledge wrote: > So pos is bad because it's abbreviated. And I suspect we may have > problems with renaming windowState to state, because in QML that > usually refers to the state-machine state (driving animated > transitions etc.) Should we be inconsistent and leave that one as > windowState then? The name "state" is also very generic, so that's a candidate for not being used. We need descriptive names that convey meaning and aren't ambiguous. That one doesn't fit the bill. For example, QAbstractSocket has a state() property that returns an enum of type SocketStatus. To be honest, socketState() might have made more sense, as QSslSocket later overloads that with the "encryption state". Whether the other ones like "title" make sense without the "window" prefix, you need to decide. Is it unambiguous what that title is referring to? How about the modality? Do people usually refer to it as the "window modality"? Food for thought. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
On 19 October 2012 21:56, Oswald Buddenhagen wrote: > On Fri, Oct 19, 2012 at 04:46:01PM +, Rutledge Shawn wrote: >> QWindow has properties like windowTitle, windowIcon, windowModality, >> windowState and so on, which are named that way to be familiar to users of >> QWidget. >> > correct. http://doc.qt.digia.com/qq/qq13-apis.html#theartofnaming , > "general naming rules". > > i don't see why this shouldn't apply equally to qml. if it poses a > problem in a particular case, it is most likely indicative of a serious > API issue. So pos is bad because it's abbreviated. And I suspect we may have problems with renaming windowState to state, because in QML that usually refers to the state-machine state (driving animated transitions etc.) Should we be inconsistent and leave that one as windowState then? ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
On Fri, Oct 19, 2012 at 04:46:01PM +, Rutledge Shawn wrote: > QWindow has properties like windowTitle, windowIcon, windowModality, > windowState and so on, which are named that way to be familiar to users of > QWidget. > correct. http://doc.qt.digia.com/qq/qq13-apis.html#theartofnaming , "general naming rules". i don't see why this shouldn't apply equally to qml. if it poses a problem in a particular case, it is most likely indicative of a serious API issue. regards ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
my +1 to renaming (didn't review the patches, though)
Konstantin
2012/10/19 Samuel Rødal :
> On 10/19/2012 06:46 PM, Rutledge Shawn wrote:
>> QWindow has properties like windowTitle, windowIcon, windowModality,
>> windowState and so on, which are named that way to be familiar to users of
>> QWidget. However it causes some silliness in Qt Quick: QQuickWindow
>> inherits QWindow, and that means it inherits those properties too (even
>> though they have not yet been documented). But we would like the QML API
>> for Window to be more typical, e.g.
>>
>> Window {
>> title: "the title"
>> modality: Qt.ApplicationModal
>> }
>>
>> QQuickWindow could of course add the extra properties, but then you would be
>> able to access either one, because I don't know of a way to hide an
>> inherited property. Furthermore because inherited signals don't work in
>> Q_PROPERTY declarations, it's not enough to just add
>>
>> Q_PROPERTY(Qt::WindowModality modality READ windowModality WRITE
>> setWindowModality NOTIFY windowModalityChanged)
>>
>> in order to reuse the accessors and just rename the property. There must be
>> a new signal in the subclass too. So then I have to add a setter to emit
>> the signal when the property changes. This is why it's easier to rename
>> them in QWindow; and really the naming was redundant anyway. If you have a
>> QWidget, you need to be clear what kind of title is being set, because there
>> can be an implied window; but if you have a QWindow it's already clear.
>>
>> So that change resulted in the following patches so far:
>>
>> https://codereview.qt-project.org/#change,37763
>> https://codereview.qt-project.org/#change,37764
>> https://codereview.qt-project.org/#change,37765
>> https://codereview.qt-project.org/#change,37766
>> https://codereview.qt-project.org/#change,37762
>>
>> and I have one for Webkit as well. I just wondered if there are any
>> objections before we proceed with testing and trying to integrate this, or
>> if anyone sees something that I missed . I will be doing further testing to
>> make sure we get the desired result in Qt Quick.
>
> Agreed, this makes sense from the C++ API side as well,
> window->setModality() is nicer API-wise than
> window->setWindowModality(). It's less redundant and also more
> consistent, since we don't do window->setWindowPos() to set the position
> of the window for example.
>
> Are people in general ok with fixing API warts in new APIs like these
> while we still have the chance?
>
> --
> Samuel
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] renaming all QWindow properties that have "window" in them
On 10/19/2012 06:46 PM, Rutledge Shawn wrote:
> QWindow has properties like windowTitle, windowIcon, windowModality,
> windowState and so on, which are named that way to be familiar to users of
> QWidget. However it causes some silliness in Qt Quick: QQuickWindow inherits
> QWindow, and that means it inherits those properties too (even though they
> have not yet been documented). But we would like the QML API for Window to
> be more typical, e.g.
>
> Window {
> title: "the title"
> modality: Qt.ApplicationModal
> }
>
> QQuickWindow could of course add the extra properties, but then you would be
> able to access either one, because I don't know of a way to hide an inherited
> property. Furthermore because inherited signals don't work in Q_PROPERTY
> declarations, it's not enough to just add
>
> Q_PROPERTY(Qt::WindowModality modality READ windowModality WRITE
> setWindowModality NOTIFY windowModalityChanged)
>
> in order to reuse the accessors and just rename the property. There must be
> a new signal in the subclass too. So then I have to add a setter to emit the
> signal when the property changes. This is why it's easier to rename them in
> QWindow; and really the naming was redundant anyway. If you have a QWidget,
> you need to be clear what kind of title is being set, because there can be an
> implied window; but if you have a QWindow it's already clear.
>
> So that change resulted in the following patches so far:
>
> https://codereview.qt-project.org/#change,37763
> https://codereview.qt-project.org/#change,37764
> https://codereview.qt-project.org/#change,37765
> https://codereview.qt-project.org/#change,37766
> https://codereview.qt-project.org/#change,37762
>
> and I have one for Webkit as well. I just wondered if there are any
> objections before we proceed with testing and trying to integrate this, or if
> anyone sees something that I missed. I will be doing further testing to
> make sure we get the desired result in Qt Quick.
Agreed, this makes sense from the C++ API side as well,
window->setModality() is nicer API-wise than
window->setWindowModality(). It's less redundant and also more
consistent, since we don't do window->setWindowPos() to set the position
of the window for example.
Are people in general ok with fixing API warts in new APIs like these
while we still have the chance?
--
Samuel
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
