Re: Review Request 115723: Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/ --- (Updated Feb. 15, 2014, 12:21 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Repository: kio Description --- Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on We cannot properly determine the windowing system platform on unix like systems in kprotocolmanager as it's not linking gui. Thus we don't know whether we are on X11 or Wayland and there is no proper way to figure it out, because both DISPLAY and WAYLAND_DISPLAY could be defined. As a solution we just force the platform to be always X11 when we are on unix like systems (modulo mac). Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115723/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115723: Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/#review49816 --- This review has been submitted with commit b4180cb753910b3ba2078267f43e5a0062a5f2fd by Martin Gräßlin to branch master. - Commit Hook On Feb. 13, 2014, 1:03 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/ --- (Updated Feb. 13, 2014, 1:03 p.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Repository: kio Description --- Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on We cannot properly determine the windowing system platform on unix like systems in kprotocolmanager as it's not linking gui. Thus we don't know whether we are on X11 or Wayland and there is no proper way to figure it out, because both DISPLAY and WAYLAND_DISPLAY could be defined. As a solution we just force the platform to be always X11 when we are on unix like systems (modulo mac). Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115723/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115723: Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/#review49709 --- Ship it! That seems like a reasonable compromise to me. I wonder what Mozilla/Chromium send when the windowing system is something other than X11. Probably the same thing. - Dawit Alemayehu On Feb. 13, 2014, 1:03 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/ --- (Updated Feb. 13, 2014, 1:03 p.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Repository: kio Description --- Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on We cannot properly determine the windowing system platform on unix like systems in kprotocolmanager as it's not linking gui. Thus we don't know whether we are on X11 or Wayland and there is no proper way to figure it out, because both DISPLAY and WAYLAND_DISPLAY could be defined. As a solution we just force the platform to be always X11 when we are on unix like systems (modulo mac). Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115723/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115723: Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/#review49747 --- src/core/kprotocolmanager.cpp https://git.reviewboard.kde.org/r/115723/#comment34981 Maybe move the Q_OS_MAC up as the first check, then there is no need for !defined(Q_OS_DARWIN) - Alexander Richardson On Feb. 13, 2014, 2:03 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/ --- (Updated Feb. 13, 2014, 2:03 p.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Repository: kio Description --- Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on We cannot properly determine the windowing system platform on unix like systems in kprotocolmanager as it's not linking gui. Thus we don't know whether we are on X11 or Wayland and there is no proper way to figure it out, because both DISPLAY and WAYLAND_DISPLAY could be defined. As a solution we just force the platform to be always X11 when we are on unix like systems (modulo mac). Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115723/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115723: Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on
On Feb. 13, 2014, 11:27 p.m., Alexander Richardson wrote: src/core/kprotocolmanager.cpp, line 690 https://git.reviewboard.kde.org/r/115723/diff/1/?file=243874#file243874line690 Maybe move the Q_OS_MAC up as the first check, then there is no need for !defined(Q_OS_DARWIN) I considered that when doing the change, but decided against it as * I don't want to reorder * are afraid of introducing a bug (after all I can only compile check one variant and the CI system as well) * it's a common pattern I have seen in kde-workspaces So overall I considered just changing the first condition as the better solution. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/#review49747 --- On Feb. 13, 2014, 2:03 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115723/ --- (Updated Feb. 13, 2014, 2:03 p.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Repository: kio Description --- Use Q_OS_UNIX instead of HAVE_X11 to determine the platform we are on We cannot properly determine the windowing system platform on unix like systems in kprotocolmanager as it's not linking gui. Thus we don't know whether we are on X11 or Wayland and there is no proper way to figure it out, because both DISPLAY and WAYLAND_DISPLAY could be defined. As a solution we just force the platform to be always X11 when we are on unix like systems (modulo mac). Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115723/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
