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

Tolerate tool name being None #1250

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ColinKinloch
Copy link

@ColinKinloch ColinKinloch commented Mar 12, 2024

@odysseywestra
Copy link
Member

Would it be better to give it an "Unknown Device" kind of output instead of None? That way the user knows there's a device connected, just there not tool_name assigned to it? If that doesn't matter then I'm fine pulling it in as is. @AesaraB what are your thoughts?

@jtojnar
Copy link
Contributor

jtojnar commented Mar 12, 2024

Would it be better to give it an "Unknown Device" kind of output instead of None? That way the user knows there's a device connected, just there not tool_name assigned to it?

It does not look like the name is used for anything user facing here:

Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Would be nice to include the link to the wayland stuff in the commit message so that the reasoning is accessible in the future (e.g. with git blame).

@odysseywestra
Copy link
Member

Then I'm guessing the right course of actions instead of none is to generate a consistent random device ID then?

@jtojnar
Copy link
Contributor

jtojnar commented Mar 12, 2024

Well, random id would not be able to match anything because it would change between storing the brush and matching it against device. Maybe we could use the device id but that would not work when there are multiple devices with same id – I assume pencil and eraser would have the same id. And it is optional too.

Edit: Actually, the linked part of the spec does not sound relevant to me, I would expect zwp_tablet_tool_v2 to be relevant instead of zwp_tablet_v2.

Edit 2: Right, looks like GTK 4 splits them correctly but GTK 3, preceding Wayland, mixes Devices and tools somewhat. But GdkDeviceTool did appear in GTK 3.22. Maybe we should port to it and use the serial id. This would also make porting to GTK 4 easier.

@odysseywestra
Copy link
Member

odysseywestra commented Mar 12, 2024

Yeah that sounds like that should be a better way to go then just putting an exception for a empty device name.

So I guess in terms of this pull request we would need to close it, and then open up another issue regarding this for porting to gtk4.

Unless @ColinKinloch, are willing to put in the work to address it?

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

Successfully merging this pull request may close these issues.

None yet

3 participants