Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add wl_resource_get_client to lib #39

Merged
merged 1 commit into from
Jul 24, 2022

Conversation

m-col
Copy link
Collaborator

@m-col m-col commented Jul 2, 2022

I used this for qtile/qtile#3663

Is there a more appropriate way to get access to this function, considering it isn't used by pywayland directly, or is this fine?

@flacjacket
Copy link
Owner

I'm fine with this. Is this something that we could wrap in a pywayland.server.Client and use something like wlroots.Ptr to compare this to the other client? We'd need to come up with a way to create a Client without it being created. If that makes things too hard, we can stick with this to get the functionality in and fix it up as we find better ways to structure this.

@m-col
Copy link
Collaborator Author

m-col commented Jul 2, 2022

Something equivalent to this (though the construction could be moved to __init__ if we make its current arguments and creation optional)?:

    @classmethod
    def from_resource(cls, resource: Resource) -> Client:
        """Look up the corresponding wl_client for a wl_resource

        :param resource: The wl_resource
        :type resource: pywayland.protocol_core.Resource
        :returns:
            A `Client` instance.
        """
        client = Client.__new__()
        client._ptr = lib.wl_resource_get_client(res_ptr)
        client._display = None
        return client

That would be nice. Currently Client.__init__ creates a new wl_client, rather than allowing for representation of existing wl_clients, but that would be useful for cases like this.

@flacjacket
Copy link
Owner

Right, something like that!

@m-col m-col force-pushed the wl_resource_get_client branch from 807a732 to 69a39aa Compare July 23, 2022 10:28
@m-col
Copy link
Collaborator Author

m-col commented Jul 23, 2022

Commit message for info on the updates:

This provides 2 alternative ways to construct Client instances:

- via the `from_resource` class method, from a wl_resource pointer. This
  accepts a pointer cdata rather than a `Resource` instance, as should
  be feasible to use this method without requiring an interface to
  implement the `Resource` generic for it. E.g. the layer shell protocol
  doesn't have a `Resource` class and this is fine; the requirement
  would disable usage of `Client.from_resource` from layer shell
  components.
- by passing `ptr=` to `Client` directly to represent an existing
  wl_client rather than creating a fresh one.

@m-col
Copy link
Collaborator Author

m-col commented Jul 23, 2022

This is causing an interesting mypy error:

pywayland/server/client.py:157: error: Module has no attribute "wl_resource_get_client"; maybe "wl_resource_get_id", "wl_resource_get_version", or "wl_resource_get_user_data"?  [attr-defined]

The new C function added to the ffi lib isn't found by mypy so mypy is complaining about the absent attr. Any ideas how this can be resolved? Not sure I've seen this before with all of the ffi modules we use!

@flacjacket
Copy link
Owner

Right, you can add the stubs to pywayland/_ffi/lib.pyi.

@m-col m-col force-pushed the wl_resource_get_client branch from 69a39aa to c526c35 Compare July 24, 2022 09:48
This provides 2 alternative ways to construct Client instances:

- via the `from_resource` class method, from a wl_resource pointer. This
  accepts a pointer cdata rather than a `Resource` instance, as should
  be feasible to use this method without requiring an interface to
  implement the `Resource` generic for it. E.g. the layer shell protocol
  doesn't have a `Resource` class and this is fine; the requirement
  would disable usage of `Client.from_resource` from layer shell
  components.
- by passing `ptr=` to `Client` directly to represent an existing
  wl_client rather than creating a fresh one.
@m-col m-col force-pushed the wl_resource_get_client branch from c526c35 to e70aef5 Compare July 24, 2022 09:52
Copy link
Owner

@flacjacket flacjacket left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@flacjacket flacjacket merged commit 514a0ea into flacjacket:main Jul 24, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants