[Bug 74030] CM should know the account path of its connections

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=74030

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
Yes, please do add a getter. You've effectively already made it public API so
you might as well.

Spec wording is still missing.

> + param_names = tp_protocol_dup_param_names (protocol);
> + if (tp_strv_contains ((const gchar * const *) param_names,

tp_protocol_has_param() is easier to use and more efficient.

> + strrchr (account->priv->unique_name, '/') + 1);

Oh, for the account /o/fd/Tp/gabble/jabber/blahblah you're just passing in
"blahblah" and not "gabble/jabber/blahblah"? I'd assumed it would be the whole
unique tail.

The fact that it's taken me this long to realize what you meant means the
documentation could probably be better. Explicitly mentioning an example would
help a lot, I think:

    such as "chris_40example_2ecom0" for the account whose object path is
    %TP_ACCOUNT_OBJECT_PATH_BASE + "gabble/jabber/chris_40example_2ecom0"

or

    such as "gabble/jabber/chris_40example_2ecom0" for the account whose
    object path is %TP_ACCOUNT_OBJECT_PATH_BASE +
    "gabble/jabber/chris_40example_2ecom0"

I realize it's redundant, but I'd prefer "gabble/jabber/blahblah" so we only
have two ways to talk about an account (object-path or suffix), rather than
three (object-path, semi-machine-readable suffix including CM and protocol, and
opaque suffix of the suffix).

Also, in practice I expect CMs will often be writing to
$XDG_CACHE_HOME/telepathy/gabble/jabber/blahblah or something, so it might as
well be "precomposed".

> + tp_asv_set_string (ctx->params, g_strdup ("account-path-suffix"),

When doing tricky things with hash tables and their memory-allocation models,
please write what you mean "the long way", with g_hash_table_insert(). You can
use tp_g_value_slice_new_string() to make it a bit shorter, but
tp_asv_set_string() is documented in terms of hash tables created with
tp_asv_new(), so please don't use it with a g_strdup'd key in a hash table
whose memory-allocation model differs (even though in this case it happens to
work).

Also, if you're going to rely on ctx->params being { g_strdup'd =>
slice-allocated }, please make sure to say so in the doc-comment of
mcd_account_coerce_parameters(), so future maintainers know to be careful when
changing it. Bonus points if you say which function relies on it.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.