search

sharplispers / clx

a fork of crhodes' fork of danb's fork of the CLX library, an X11 client for Common Lisp
Other
114 stars 46 forks source link

rr-get-output-info fix #198

Closed einsiedlerspiel closed 1 year ago

einsiedlerspiel commented 1 year ago

The index crtc-start in rr-get-output-info is 26 when it should be 36, resulting in mangled output for everything that depends on that index.

https://github.com/sharplispers/clx/blob/e8c9033a34d194e51d862360a6c8e4474a8e46d9/extensions/randr.lisp#L665-L693

I'd love to have a fixed version of it. Both #172 and the merged and now reversed #193 provide that. Before I open a third pull request on this i wanted to check in what the preferred fix would look like for this function.

The easiest fix is to just change the crtc-start index to 36 and change the line that gets the value back to the string-get-version (switch line 692 for 691).

The slightly more extensive fix would be to also reorder the output values to be compliant with the randr spec. Though reordering the output values might again break existing software (though I'm questioning whether anyone is using this function in its current state anyway).

I do like the version of the function implemented by #193 for being more readable, though it does reorder the output values in a way that still doesn't match the spec.

https://github.com/sharplispers/clx/blob/a85ad99ba2f7c2bbdfc57402a7c5b0c264a4ea5e/extensions/randr.lisp#L752-L808

What should be done?

einsiedlerspiel commented 1 year ago

StumpWM depends on the order of return values.

https://github.com/stumpwm/stumpwm/blob/9b5e0cfd19d0dfb18e284ac140431a2685afbe3e/head.lisp#L41-L60

And just in general changing them is probably a bad idea at this point.