[Telepathy] Proposal for changes on tp-qt4

[Olli Salli]

On Wed, Dec 10, 2008 at 4:36 PM, Andre Moreira Magalhaes
<andre.magalhaes at collabora.co.uk> wrote:
> Simon McVittie wrote:
>> On Wed, 10 Dec 2008 at 12:37:38 +0000, Andre Moreira Magalhaes wrote:
>>
>>> - Move all client code from TelepathyQt4 to TelepathyQt4/Client, this
>>> includes moving all cli-*.* files to the Client subdir and renaming
>>> them, removing the cli- prefix.
>>>
>>
>> I agree, but this is a disruptive change, so we should do it when there are
>> fewer outstanding branches (in particular, Olli's "handles" branch
>> and my "cm" branch should go in first).
>>
>> Olli wanted to reserve Client/ for the stub headers and put all real
>> code in some other subdirectory; I'm not clear why this is. Olli, can you
>> explain?
>>
> I would stick with the Client dir for the code itself, so we don't have
> code splitted in different directories. Oli??

Bah, this is acceptable to me. The current convention was born more or
less from an attempt to have a similar structure to tp-glib's (where
everything is in the single telepathy-glib/ dir, except for generated
code which is in telepathy-glib/_gen). Adding the stub pretty include
headers in the current fashion was more or less an accident - I
thought that  if I listed Client/Foo in tpqt4include_HEADERS, they
would get installed to TelepathyQt4/Client/Foo instead of
TelepathyQt4/Foo like they actually do before realizing I actually
need to have a separate tpqt4clientinclude_HEADERS target. If I had
realized them the other way around, everything would probably be in a
single dir, but I agree, if we someday happen to add service-side to
the mix, there will probably just be too many files in the single dir.

>>> - In the code generation process we are creating source files in a
>>> directory called TelepathyQt4/_gen that will be installed. As these
>>> files may and probably will be used by applications, I suggest not
>>> putting this files in the _gen dir, instead we should put them in the
>>> same dir as the helper classes but with a different nomenclature. For
>>> example the generated files for connection-manager.xml would go to
>>> TelepathyQt4/Client/connection-manager-interface.* not conflicting with
>>> the helper class called TelepathyQt4/Client/connection-manager-*. So all
>>> generated files would have a suffix -interface.*.
>>>
>>
>> The reason the _gen directory has such an ugly name is to remind library
>> users that they should never be #include'd directly. I don't consider the
>> presence and exact naming of these header files to be API; consider them
>> to be an implementation detail of the code generation process.
>>
>> In telepathy-glib the headers in _gen are not even self-contained - you
>> have to #include some more stuff to make them work. The non-generated
>> headers like <telepathy-glib/channel.h> take care of doing that, so
>> these are the only ones that library users should #include.
>> (Is it conventional in Qt to consider the "lower-case" headers not to be
>> part of the API either?)
>>
> There is no such convention, but we usually include the camel case headers.

Yes, it IS NOT intended that an user of the library includes the
headers like TelepathyQt4/_gen/cli-connection.h or even
TelepathyQt4/cli-connection.h directly, but instead just includes
headers like TelepathyQt4/Client/Connection to get everything
Connection-related, including the generated functionality (and
currently, for things like Client/DBus, this is all there is in fact).
This is reflected in the \headerfile statements in the doxygen,
indicating that the user should eg. include
TelepathyQt4/Client/Channel to get
Telepathy::Client::ChannellnterfaceGroupInterface, for example
(however, this too might be a mistake - perhaps we should have
separate pretty include headers for ChannelInterfaceGroupInterface
etc., but all of them actually including the same real header in the
end like they currently do).

The fact that non-camelcase headers are not intended to be included
directly should perhaps also be enforced more strictly, with the
pretty include headers (and implementation files) #defining
_TelepathyQt4_YOU_WERE_NOT_INCLUDED_DIRECTLY_ITS_ALRIGHT_ and the real
headers doing #ifndef that #error Hey, don't include this crap
directly, because this might go away/move/be split/merged/... etc.

>
>>> - As the generated files will also be exported I suggest creating header
>>> files for them the same way we do for the helper classes. Example:
>>> TelepathyQt4/Client/connection-manager-interface.h would have a
>>> corresponding TelepathyQt4/Client/ConnectionManagerInterface.
>>>
>>
>> We could do that, but IMO
>> <TelepathyQt4/Client/ConnectionManagerInterface> should #include
>> <TelepathyQt4/cli-connection-manager.h> (or whatever we rename it to) rather
>> than directly including the generated header; the generated headers
>> aren't guaranteed to be self-contained.
>>
> Ah good, just to make it clear, so we should have a cli-*.h for each
> generated interface we have, even if there is no implementation of it,
> so users don't include the generated header directly?

Yeah I mostly agree with this now (see previous point), however unlike
the pretty include files for each class, I think there should be a
real header file for a related group of functionality, for example all
Channel-related client-side parts (currently, there is
TelepathyQt4/cli-channel.h for this exact purpose, could be changed to
TelepathyQt4/Client/channel.h as in the previous point). Splitting the
generated interfaces into multiple headers doesn't really serve any
practical purpose I don't think, and it's natural for them to be a
part of the same doxygen module, for example (possibly with moving the
generated parts to "direct accessors" submodules as we gain more
convenience code).

>
> BR
> Andrunko
>
-- 

Br,
Olli Salli