search

FlorianRhiem / pyGLFW

Python bindings for GLFW
MIT License
233 stars 36 forks source link

The `window` argument for `glfw[GS]etClipboardString` has been deprecated in libglfw 3.3 #67

Closed papr closed 1 year ago

papr commented 1 year ago

References:

This is the code section that needs changing.

Not 100% sure if it is as simple as passing None instead of a window reference.

FlorianRhiem commented 1 year ago

Hey Pablo, thanks for reporting this. Passing None works just fine, it is interpreted as a NULL pointer, which the functions expect. So as-is, everything works but users need to pass a useless None to the functions. For glfw.get_clipboard_string I could set window to have a default value of None, but glfw.set_clipboard_string takes first window and then string as parameters, so I can't really change the parameters without either risk of breaking existing programs or making the signature worse by giving string a misleading default value. As such, I think the best course here might be to not change anything as long as GLFW also doesn't change the function signature and then adapt to the change based on the runtime library version. Do you have a better suggestion for handling this?

papr commented 1 year ago

@FlorianRhiem Thank you for taking the time to look into this! I fully understand your arguments and I agree with them.

Once libglfw releases a backward-incompatible version, you can too. I.e. no need to change the function signature just now. But maybe it would be worth looking at all newly-released libglfw deprecations and mirror them in pyGLFW with a corresponding DeprecationWarning. This way, people will know what will change should libglfw make such a backward incompatible release.

FlorianRhiem commented 1 year ago

That seems reasonable. I've added deprecation warnings in 045c451462ab4b366bbd38926164ffbb40c83286. Would you mind reviewing the code? If I didn't miss anything and it looks good to you, I'll merge it into master and release a new version.

papr commented 1 year ago

Looks good to me! Could not find any other deprecation notice in any of the recent libglfw release notes. Also, setting the stack level argument to 2 is a detail that is easily overlooked when adding deprecation warnings to one's code. 🚀

FlorianRhiem commented 1 year ago

Thanks for the review! I've merged the code into master and released v2.5.6

papr commented 1 year ago

Thank you for the fast turnaround time!