Skip to content
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.

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

QATERIAL_ENABLE_ICONS=OFF code path is broken. #150

Closed
MarekPasnikowski opened this issue Mar 27, 2024 · 4 comments · Fixed by #151
Closed

QATERIAL_ENABLE_ICONS=OFF code path is broken. #150

MarekPasnikowski opened this issue Mar 27, 2024 · 4 comments · Fixed by #151

Comments

@MarekPasnikowski
Copy link

https://github.com/OlivierLDff/Qaterial/blame/cf5a5ef064c3deaaf54faff722a808544e7a87f0/cmake/QaterialGenerateIcons.cmake#L184

The last update to the function introduced new arguments, but this change was not implemented in the ENABLE_ICONS=OFF branch.

I was able to complete the compilation with the attached change, but this patch is a blind copy-paste of code with some cuts and renames — the chance that this is the correct solution is very low.

PS: To my frustration, GitHub does not allow me to attach neither a .patch, nor a .cmake file, so I am dumping the .patch below:

From 1c6b82f51a29f184727256da40c649bc46d81e57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Pa=C5=9Bnikowski?= <[email protected]>
Date: Wed, 27 Mar 2024 10:50:08 +0100
Subject: [PATCH] Bug Fix

---
 cmake/QaterialGenerateIcons.cmake | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/cmake/QaterialGenerateIcons.cmake b/cmake/QaterialGenerateIcons.cmake
index d28d45d..3b54ae9 100644
--- a/cmake/QaterialGenerateIcons.cmake
+++ b/cmake/QaterialGenerateIcons.cmake
@@ -200,8 +200,8 @@ function(qaterial_generate_icons_class OUTPUT_FILE_HPP OUTPUT_FILE_CPP)
   else()
 
     # Generate fake Qaterial.Impl.Icons.Icons.qml
-    message(STATUS "Generate Fake ${OUTPUT_FILE}")
-    file(WRITE ${OUTPUT_FILE}
+    message(STATUS "Generate Fake ${OUTPUT_FILE_HPP}")
+    file(WRITE ${OUTPUT_FILE_HPP}
       "// Dummy file generated with CMake to mock the absence of Mdi icons.\n"
       "// Everything written here will be lost.\n\n"
       "#ifndef __QATERIAL_ICONS_HPP__\n"
@@ -221,6 +221,27 @@ function(qaterial_generate_icons_class OUTPUT_FILE_HPP OUTPUT_FILE_CPP)
       "#endif"
     )
 
-  endif()
+    # Generate fake Qaterial.Impl.Icons.Icons.qml
+    message(STATUS "Generate Fake ${OUTPUT_FILE_CPP}")
+    file(WRITE ${OUTPUT_FILE_CPP}
+      "// Dummy file generated with CMake to mock the absence of Mdi icons.\n"
+      "// Everything written here will be lost.\n\n"
+      "#ifndef __QATERIAL_ICONS_HPP__\n"
+      "#define __QATERIAL_ICONS_HPP__\n\n"
+      "#include <Qaterial/Details/Export.hpp>\n"
+      "#include <Qaterial/Details/Property.hpp>\n\n"
+      "#include <QtCore/QObject>\n\n"
+      "namespace qaterial {\n\n"
+      "class QATERIAL_API_ Icons : public QObject\n"
+      "{\n"
+      "    QATERIAL_SINGLETON_IMPL(Icons, icons, Icons);\n\n"
+      "public:\n"
+      "    Icons(QObject* parent = nullptr) : QObject(parent) {}\n\n"
+      "};\n\n"
+      "}\n\n"
+      "#endif"
+    )
+
+endif()
 
 endfunction()
-- 
2.41.0
@OlivierLDff
Copy link
Owner

You are right, as this option is not tested in CI.
Would you provide your patch as a PR?

@MarekPasnikowski
Copy link
Author

I could.

Does the dummy code look appropriate to you? I have zero knowledge of the programming languages involved here and this report is the result of my work towards packaging an application in Guix.

@OlivierLDff
Copy link
Owner

I will do the appropriate PR, and add the appropriate tests in CI. The file that is generated should be OUTPUT_FILE_HPP, and I guess an empty OUTPUT_FILE_CPP should be generated

@OlivierLDff
Copy link
Owner

#151 should fix your issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants