Fix WindowsRegistryKey for Qt 6.8.1 #154

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.

Sign up for GitHub

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

Jump to bottom

Merged

wangwenx190 merged 5 commits into stdware:main from dnlkrs:qt-6-8-1-windows-patch

Dec 6, 2024

Contributor

dnlkrs commented Dec 3, 2024

Qt changed some of their private API with version 6.8.1, which causes compilation errors.
Specifically they removed QWinRegistryKey::dwordValue(...) in favor of QWinRegistryKey::value(...)


          Fix WindowsRegistryKey for Qt 6.8.1

Contributor Author

dnlkrs commented Dec 3, 2024

Here the commit in qtbase
qt/qtbase@7a63a25

Collaborator

wangwenx190 commented Dec 5, 2024

Thanks! But I think we should use new API in user code instead of restoring the old Qt API.

wangwenx190 self-requested a review

December 5, 2024 07:43

wangwenx190 assigned dnlkrs

wangwenx190 added the enhancement label


          Use new Qt Private API instead of restoring the old version

b04346d

Contributor Author

dnlkrs commented Dec 5, 2024 •

edited

Loading

Yes this was my first thought aswell, but I can not test Qt versions below 5.14.0

qwindowkit/src/core/qwindowkit_windows.h

Line 114 in ba8e8a3

#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)

For the most recent commit I confirmed compilation with these Qt versions (Windows, MinGW):

6.2.13
6.7.3
6.8.1

wangwenx190 requested changes

View reviewed changes

Collaborator

wangwenx190 left a comment

Thanks! Only minor tweaks needed.

src/core/qwindowkit_windows.cpp Outdated

                       return qMakePair(value, ok);
                   }
+                  template<>

Collaborator

wangwenx190 Dec 5, 2024

template implementations should be placed in header files

src/core/qwindowkit_windows.cpp Outdated

+                  template<>
+                  std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {
+                      auto dv = dwordValue(subKey);

Collaborator

wangwenx190 Dec 5, 2024

can be const

src/core/qwindowkit_windows.cpp Outdated

+                  template<>
+                  std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {
+                      auto dv = dwordValue(subKey);
+                      if(!dv.second) {

Collaborator

wangwenx190 Dec 5, 2024

space after if

src/core/qwindowkit_windows.cpp Outdated

+                      return dv.first;
+                  }
+              #elif QT_VERSION < QT_VERSION_CHECK(6, 8, 1)

Collaborator

wangwenx190 Dec 5, 2024

empty line not needed

src/core/qwindowkit_windows.cpp Outdated

+                  {
+                  }
+                  template<>

Collaborator

wangwenx190 Dec 5, 2024

ditto, place this in header file

src/core/qwindowkit_windows.cpp Outdated

+                  template<>
+                  std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {
+                      auto dv = dwordValue(subKey);

Collaborator

wangwenx190 Dec 5, 2024

ditto, can be const

src/core/qwindowkit_windows.cpp Outdated

+                  template<>
+                  std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {
+                      auto dv = dwordValue(subKey);
+                      if(!dv.second) {

Collaborator

wangwenx190 Dec 5, 2024

ditto, space after if

src/core/qwindowkit_windows.cpp Outdated

               #endif
-              }

                
                    No newline at end of file
+              }

Collaborator

wangwenx190 Dec 5, 2024

something that should not be changed

src/core/qwindowkit_windows.h Outdated

                       QPair<DWORD, bool> dwordValue(QStringView subKey) const;
+                      template<typename T>
+                      std::optional<T> value(QStringView subKey) const { return {}; }

Collaborator

wangwenx190 Dec 5, 2024

I see you don't want this function to be called directly, so we can use static_cast(false, "xxx") to ensure this.

src/core/qwindowkit_windows.h Outdated

+                      explicit WindowsRegistryKey(HKEY parentHandle, QStringView subKey, REGSAM permissions = KEY_READ, REGSAM access = 0);
+                      template<typename T>
+                      std::optional<T> value(QStringView subKey) const { return {}; }

Collaborator

wangwenx190 Dec 5, 2024

ditto, use static_assert here


          Applied review changes

- Moved template specializations to header, had to add inline for this to work
- Added const qualifiers
- Added space after if
- Removed unneeded new lines
- Removed new lines inserted by the IDE
- Removed general template implementation, static asserts did not compile (due to MOC i think). Using the function with any other type than DWORD will still cause a compilation error

dnlkrs requested a review from wangwenx190

December 5, 2024 14:07

wangwenx190 requested changes

View reviewed changes

Collaborator

wangwenx190 left a comment

Sorry, you may misunderstand my words. Please apply some more minor tweaks. Really thanks!

src/core/qwindowkit_windows.h Outdated

                       QPair<DWORD, bool> dwordValue(QStringView subKey) const;
+                      template<typename T>
+                      const std::optional<T> value(QStringView subKey) const;

Collaborator

wangwenx190 Dec 6, 2024

I didn't mean the return value should be const, sorry for my misleading word. You can remove the const keyword from the return value

src/core/qwindowkit_windows.h Outdated

                   }
+                  template<>
+                  inline const std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {

Collaborator

wangwenx190 Dec 6, 2024

Please remove this const keyword

src/core/qwindowkit_windows.h Outdated

+                  template<>
+                  inline const std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {
+                      auto dv = dwordValue(subKey);

Collaborator

wangwenx190 Dec 6, 2024

I was meant to make this "dv" const, but never mind, const or not doesn't really matter.

src/core/qwindowkit_windows.h Outdated

+                      explicit WindowsRegistryKey(HKEY parentHandle, QStringView subKey, REGSAM permissions = KEY_READ, REGSAM access = 0);
+                      template<typename T>
+                      inline const std::optional<T> value(QStringView subKey) const;

Collaborator

wangwenx190 Dec 6, 2024

same here, please remove the const keyword

src/core/qwindowkit_windows.h Outdated

+                  };
+                  template<>
+                  inline const std::optional<DWORD> WindowsRegistryKey::value(QStringView subKey) const {

Collaborator

wangwenx190 Dec 6, 2024

remove const

src/core/qwindowkit_windows.h Outdated

               #endif
-              #endif // QWINDOWKIT_WINDOWS_H
+              #endif // QWINDOWKIT_WINDOWS_H

                
                    No newline at end of file

Collaborator

wangwenx190 Dec 6, 2024

It seems the line ending is changed? Could you please restore it?

dnlkrs added 2 commits

December 6, 2024 14:39


          Fix const

e56a4d8


          Fix line ending

291a52d

dnlkrs requested a review from wangwenx190

December 6, 2024 13:46

Collaborator

wangwenx190 commented Dec 6, 2024

Thanks!

wangwenx190 merged commit 91274c5 into stdware:main

GerritRingMirror pushed a commit to savoirfairelinux/jami-client-qt that referenced this pull request


          QWK: fix build issue with version 6.8 of Qt

6b70ffc

This commit updates QWindowKit in order to fix the following build issue
on Qt 6.8: stdware/qwindowkit#154

GitLab: #1944
Change-Id: Iab82514b51577148ff5dc1ba382f8bb517c94f8a

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

Labels