http://bugs.freedesktop.org/show_bug.cgi?id=20033
Simon McVittie <simon.mcvit...@collabora.co.uk> changed:
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|telepa...@lists.freedesktop.|andru...@gmail.com
|org |
QAContact| |telepathy-
| |b...@lists.freedesktop.org
--- Comment #1 from Simon McVittie <simon.mcvit...@collabora.co.uk> 2009-07-23
08:55:34 PST ---
Comments on andrunko's branch, apart from some that I already told him on IRC.
+ if (group.isEmpty() || group.trimmed().isEmpty()) {
+ return new PendingFailure(this, TELEPATHY_ERROR_INVALID_ARGUMENT,
+ "Group must be a non-empty UTF-8 string");
+ }
This is wrong - nothing says that CMs don't support groups with the empty name,
or groups whose name is entirely whitespace. Don't do this check - if the CM
doesn't like your group name, it is the CM's responsibility to fail.
+ // TODO Check here. Spec states that in order to create an empty group
+ // RequestHandles with HandleType 'HandleTypeGroup' and the UTF-8
name
+ // of the group should be called, but this just does not work.
+ // Using CreateChannel does the job, so let's use it.
+ /*
+ return connection()->requestHandles(HandleTypeGroup,
+ QStringList() << group);
+ */
You are right that this is both insufficient and unnecessary. The old way to
make a group was RequestHandles() followed by RequestChannel(), but
CreateChannel() does that for us.
+/**
+ * Attempt to add an user-defined contact list group named \a group.
+ *
+ * On some protocols (e.g. XMPP) empty groups are not represented on the
server,
+ * so disconnecting from the server and reconnecting might cause empty groups
to
+ * vanish.
+ *
+ * \param group Group name.
+ * \return A pending operation which will return when an attempt has been made
+ * to add an user-defined contact list group.
+ * \sa groupAdded(), groupAddContacts()
+ */
+PendingOperation *ContactManager::addGroup(const QString &group)
+ return connection()->createChannel(request);
I think this function should use ensureChannel, so it succeeds if the group
already exists.
Do we actually need API to create a group at all, or should the C++ API to
create groups just be "add a contact to the desired group"?
+/**
+ * Attempt to remove an user-defined contact list group named \a group.
+ *
+ * User-defined contact list groups may only be deleted if the group is
+ * already empty.
If the user has explicitly called a method called removeGroup(), wouldn't it be
friendlier to remove all the members and then close the channel?
(The restriction on deleting non-empty groups via the D-Bus API is to stop
naive clients, like MC 4, accidentally deleting all your groups because they
don't know where to dispatch them... it's not desirable here.)
-void ContactManager::setContactListChannels(
+void ContactManager::setContactListsChannels(
Revert this, please. The old version is correct English.
+/**
+ * Return a list of user-defined contact list groups the contact belongs.
+ *
+ * This method requires Connection::FeatureRosterGroups to be enabled.
Can we have a feature on Contact objects, that implicitly enables that
Connection feature the first time it is enable on any Contact?
I think a setGroups() method would be good to have - in the current D-Bus API
it would become an inefficient series of calls to group{Add,Remove}Contacts,
but in a future (more Contact-centric) D-Bus API, it would be a single D-Bus
call.
--
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
_______________________________________________
telepathy mailing list
telepathy@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/telepathy
