Bug 31818 - Document and test argXpath more thoroughly, and support paths
Summary: Document and test argXpath more thoroughly, and support paths
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: Other All
: medium normal
Assignee: Will Thompson
QA Contact: John (J5) Palmieri
URL: http://git.collabora.co.uk/?p=user/wj...
Whiteboard: r+ for 1.5, smcv to merge
Keywords: patch
Depends on:
Blocks: 24317
  Show dependency treegraph
 
Reported: 2010-11-21 09:06 UTC by Will Thompson
Modified: 2011-04-07 09:29 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Document when argNpath was added, for completeness (1.30 KB, patch)
2011-04-07 07:30 UTC, Simon McVittie
Details | Splinter Review

Description Will Thompson 2010-11-21 09:06:28 UTC
I discovered that argXpath matches don't actually do what I thought they did, and that they aren't really well tested, and that they don't actually work on path arguments.

So here's a branch that records the explanation Ryan gave me in Cambridge, as well as adding some tests and making messages with suitable 'o' arguments match (as well as 's' arguments).
Comment 1 Simon McVittie 2010-11-22 04:38:18 UTC
I think it's worth saying explicitly in the Specification that argN works on strings, and argNpath works on strings or object paths. Other than that, I like this branch.
Comment 2 Simon McVittie 2010-11-22 04:54:48 UTC
(In reply to comment #1)
> I think it's worth saying explicitly in the Specification that argN works on
> strings, and argNpath works on strings or object paths.

Perhaps this could be phrased by replacing this bit:

> Argument path matches provide a specialised form of wildcard
> matching for path-like namespaces. As with normal argument matches,
> if the argument is exactly equal to the string given in the match

with something like "... path-like namespaces. They can match arguments whose type is either STRING or OBJECT_PATH. As with normal ..."

Relatedly, the argN part says:

> As of this time only string arguments can be matched.

which should probably be "... only STRING arguments can be matched in this way", to be consistent with the use of capitalized type names when describing the message format, and to reflect that yes you can match non-STRINGs now, just not with argN.

(I'm suggesting capitalization to try to avoid people thinking that maybe "string" includes STRING, OBJECT_PATH and SIGNATURE, like g_variant_get_string does.)
Comment 3 Will Thompson 2010-11-23 02:54:19 UTC
I've added roughly that wording:

<http://git.collabora.co.uk/?p=user/wjt/dbus.git;a=commitdiff;h=2f618faa2d1fa2f4235c381255fabb6b58557a10>
Comment 4 Simon McVittie 2010-12-07 09:33:09 UTC
Looks good to me!
Comment 5 Simon McVittie 2011-01-06 10:25:39 UTC
I think this would be OK to put in 1.4.x, with a note that dbus-daemon/libdbus 1.5.x or later is needed for this to work.

(Almost as though dbus-spec and libdbus were distinct projects...)
Comment 6 Simon McVittie 2011-02-18 06:19:07 UTC
I hear Will still wants to change the semantics a bit, so, removing the patch tag.
Comment 7 Simon McVittie 2011-04-07 07:26:29 UTC
I propose to merge this as part of Bug #24317, if not before.
Comment 8 Simon McVittie 2011-04-07 07:30:45 UTC
Created attachment 45378 [details] [review]
Document when argNpath was added, for completeness

I'd like to add this patch too.
Comment 9 Simon McVittie 2011-04-07 09:29:30 UTC
Fixed in git for 1.5.0


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.