fvogt added a comment.
In D17122#395704 <https://phabricator.kde.org/D17122#395704>, @romangg wrote: > In D17122#395696 <https://phabricator.kde.org/D17122#395696>, @fvogt wrote: > > > In D17122#395537 <https://phabricator.kde.org/D17122#395537>, @romangg wrote: > > > > > I don't think we need a second variable effectiveSocketName. Just test if socketName is empty. If it is call wl_display_add_socket_auto, otherwise call wl_display_add_socket. > > > > > > If `socketName` is overwritten after using `wl_display_add_socket_auto`, it's not possible to call `start` twice without resetting `socketName` again. > > > > If the actual socket's name is not written to any variable, it's impossible to set `WAYLAND_DISPLAY` correctly in KWin. > > > > The new variable is used to be fully API compatible except if `socketName` was explicitly set to an empty string. > > > Is a Display object meant to be started and terminated more than once? But ok, let's make sure. Not sure, but it's technically possible and I don't see anything discouraging it. >>> According to description some autotest fails. Which one exactly? >> >> testSocketName as the default value of socketName changed. >> >> FAIL! : TestWaylandServerDisplay::testSocketName() Compared values are not the same >> Actual (display.socketName()) : "" >> Expected (QStringLiteral("wayland-0")): "wayland-0" >> Loc: [/home/fabian/kderepos/kwayland/autotests/server/test_display.cpp(54)] > > Is there an argument against just checking on empty string at this location? Since the socket name is not yet set in line 54, there shouldn't be one. The argument is that the default value changed, which is technically an ABI break - that's why this is an RFC. If you say that it's fine, I'll do the change and remove the RFC. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D17122 To: fvogt, #kwin, #plasma Cc: romangg, kde-frameworks-devel, michaelh, ngraham, bruns
