Code review comment for lp:~mir-team/mir/cursor-spike-phase-2-resubmit

(1) I think "Configuration" is a vague word with respect to MirCursorConfiguration, and find it confusing. Looking to the source for an explanation, I find a comment which is also confusing:

158 +// Cursor parameterization. May grow to include RGBA cursors, etc...
159 +struct MirCursorConfiguration {
160 + std::string name;
161 +};

We already support colour cursors(!?), so can you explain what the struct might grow to represent? Then it might be easier to come up with a more appropriate name/structure.

(2) Very minor issue: This function name is inconsistent with the existing convention of:
     mir_OBJECT_OPERATION(OBJECT x, ...)
64 +MirCursorConfiguration *mir_cursor_configuration_from_name(char const* name);
Although maintaining complete consistency can yield even more awkward names. So I'm not sure if it should be changed at all.

(3) This inline documentation needs elaboration (like a URL of X docs). There's presently not enough information provided in the header/docs to really use the function...
59 + * follow XCursor naming conventions.

(4) Inappropriate function name:
98 +MirWaitHandle* mir_surface_configure_cursor(MirSurface *surface, MirCursorConfiguration const* parameters);
The verb "configure" implies you're changing the cursor object, which is not true. More accurate would be "mir_surface_set_cursor".

(5) Per previous discussions I don't think this is correct for common toolkit usage:
405 +message CursorSetting {
406 + required SurfaceId surfaceid = 1;
407 + // No name is interpreted as disabled cursor.
408 + optional string name = 2;
409 +}
Hiding the cursor should probably not change its appearance for when it becomes visible again. And simply making the cursor invisible will lead to bugs where it sounds like the cursor still exists and is clickable on things while invisible.

The big issue is (1). We either need some explanation of what a "MirCursorConfiguration" is, or a clearer name like "MirCursor" which would already be understood.