D27338: Provide an initial implementation for input-method-unstable-v1

[Vlad Zahorodnii]

zzag added inline comments.

INLINE COMMENTS

> inputmethod_interface.cpp:128-129
> +        wl_resource_destroy(resource->handle);
> +        if (resourceMap().isEmpty())
> +            q->deleteLater();
> +    }

Destroy `q` in zwp_input_method_context_v1_destroy_resource(). In the destructor request, we usually want to destroy only the resource.

> inputmethod_interface.cpp:181
> +
> +class Q_DECL_HIDDEN InputPanelSurfaceInterface::Private : public QtWaylandServer::zwp_input_panel_surface_v1
> +{

We need to destroy the wl_resource for zwp_input_panel_surface_v1 when the associated surface is destroyed.

> apol wrote in inputmethod_interface.cpp:179
> I don't know, just looked at it and it doesn't seem that useful?

zwp_input_panel_surface_v1 defines a surface role (based on weston code) so it should be a subclass of SurfaceRole. In long term, we want to do something like

  SurfaceRole *role = SurfaceRole::get(surface);
  if (role) {
      wl_resource_post_error(resource->handle, ZMY_SHELL_SURFACE_ERROR_ROLE,
                             "Cannot reassign surface role"); // DIE!
      return;
  }

> apol wrote in inputmethod_interface.cpp:199-200
> I would rather not, otherwise when implementing private members they read like local variables.

Frameworks' policies have no a single word about this so I guess it's okay(?) to put `m_`. I asked to drop `m_` because Qt folks seem to prefer not to put `m_` in private classes.

I would appreciate if you bring this topic up to the discussion in #kde-frameworks-devel since we're not consistent with naming in private classes.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27338

To: apol, #kwin, #frameworks
Cc: zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200326/7c429c3b/attachment-0001.html>