Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Request and event naming conflict in the cosmic workspace protocol #36

Open
morr0ne opened this issue Sep 24, 2024 · 3 comments · May be fixed by #38
Open

Request and event naming conflict in the cosmic workspace protocol #36

morr0ne opened this issue Sep 24, 2024 · 3 comments · May be fixed by #38

Comments

@morr0ne
Copy link

morr0ne commented Sep 24, 2024

I recently encountered an issue with the current implementation of the cosmic workspace protocol where a request and an event share the same name: remove. This came up while generating the protocol for my Wayland library, waynest. My library currently doesn't handle such edge cases because it implements protocols as traits, which causes an error when names are duplicated. While I recognize this is something I need to address on my end, I believe it also raises broader questions about the protocol itself.

From my understanding, this does not constitute a direct protocol violation, or at least none that I could find in the specifications. However, it seems to be a generally followed (perhaps unwritten) convention that requests and events should have unique names. Out of the 106 protocols I generate (full list here), the Cosmic Workspace Protocol is the only one exhibiting this naming overlap.

Beyond my specific use case, this could present broader challenges, such as:

  • Debuggability: It’s harder to quickly identify which remove is being referenced during debugging.
  • Readability: It can create confusion when reading documentation, as identical names may imply identical behavior.
  • Code Maintainability: It adds complexity when handling differences between the request and event, making the code more awkward to work with.

I propose renaming the server-emitted event since its description suggests that it represents a workspace being destroyed, which might be better conveyed with a name distinct from remove.

Of course, this is just my perspective, and I’m primarily opening this issue to foster discussion. I recognize there might be other protocols that behave similarly that I haven’t encountered, but as Cosmic is relatively new, aligning with the established practices of other protocols could be beneficial.

@Drakulix
Copy link
Member

Yeah, I tend to agree with these point, this probably happened on accident.

I am not 100% sure how to best solve this situation though. We have quite a few components dealing with cosmic-protocols, updating them in lock-step is quite the hassle.

I would rather deprecate either the event or the request, but given how abi stability works for wayland protocols, that doesn't resolve the naming conflict.

An alternative would be to role out a v2 of said protocol in parallel until every component has moved over, we have done in the past so for our screencopy protocol for example, but I am not sure I would go through that hassle for a simple naming change.

So if nobody has a better suggestion, I guess this simply comes on the pile of "stuff to fix, if we ever do a v2 before ext-workspace lands", which is probably not a very satisfying solution to your problem. :S

In any case I would encourage you to make a PR to https://gitlab.freedesktop.org/wayland/wayland and amend the spec to forbid this. That way we can at least get a discussion going upstream to prevent this in the future.

@morr0ne
Copy link
Author

morr0ne commented Oct 5, 2024

It looks like the best way forward is to roll out a v2 of the protocol and gradually update the different parts of the ecosystem. It’s a bit of a hassle, but probably the most effective solution. I’ve opened #38 to start tackling it.

I’ll also be opening a PR against the Wayland protocol to help make sure this doesn’t happen again.

@morr0ne morr0ne linked a pull request Oct 5, 2024 that will close this issue
@morr0ne
Copy link
Author

morr0ne commented Oct 5, 2024

I opened a related pr on the freedesktop gitlab

https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants