JonMagon / KDiskMark

A simple open-source disk benchmark tool for Linux distros
GNU General Public License v3.0
1.04k stars 43 forks source link

Add command line argoment to allow more than single instance of app running #112

Open Mart-Bogdan opened 1 year ago

Mart-Bogdan commented 1 year ago

I've noticed in Readme that SingleApplication is required dependency.

And I confirmed it in code of Main and by program behavior, that I can't launch more then one instance of app.

I don't see why. I've used to similar apps on windows that don't have this constraint.

You just launch any instances you like.

So it is useful to bench two different drives in one go, and visually compare them/make screenshot of two windows.

Mart-Bogdan commented 1 year ago

P.S. IDK why it has bug label, but I wasn't able to change labels upon issue creation.

Mart-Bogdan commented 1 year ago

Would it be ok for me to create PR with something along this lines:

diff --git a/src/main.cpp b/src/main.cpp
index e300159..422d6c8 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -5,6 +5,8 @@
 #include "singleapplication.h"
 #include "cmake.h"

+bool use_multi_instance(int argc, char *argv[]);
+
 int main(int argc, char *argv[])
 {
     QCoreApplication::setApplicationName(QStringLiteral(PROJECT_NAME));
@@ -13,7 +15,12 @@ int main(int argc, char *argv[])

     QApplication::setAttribute(Qt::AA_EnableHighDpiScaling);

-    SingleApplication a(argc, argv);
+    std::unique_ptr<QApplication> a;
+    if (use_multi_instance(argc, argv)) {
+        a = std::make_unique<QApplication>(argc, argv);
+    } else {
+        a = std::make_unique<SingleApplication>(argc, argv);
+    }

     AppSettings().setupLocalization();

@@ -21,5 +28,17 @@ int main(int argc, char *argv[])
     w.setFixedSize(w.size());
     w.show();

-    return a.exec();
+    return a->exec();
 }
+
+const std::string MULTI_INSTANCE_ARG = "--multi-instance";
+
+bool use_multi_instance(int argc, char *argv[])
+{
+    for (int i = 0; i < argc; ++i) {
+        if (argv[i] == MULTI_INSTANCE_ARG) {
+            return true;
+        }
+    }
+    return false;
+}
\ No newline at end of file

Command swith to be decided. And sorry for code stype. I've have to look at style used across the project etc.

But that's just a a sketch.

As I understand: before we created QApplication instance we can't use Qt's facility to parse arguments, so have to do it manually.

Mart-Bogdan commented 1 year ago

Turns out this solution is better:

diff --git a/src/main.cpp b/src/main.cpp
index e300159..872bb6d 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -5,6 +5,8 @@
 #include "singleapplication.h"
 #include "cmake.h"

+bool use_multi_instance(int argc, char *argv[]);
+
 int main(int argc, char *argv[])
 {
     QCoreApplication::setApplicationName(QStringLiteral(PROJECT_NAME));
@@ -13,7 +15,7 @@ int main(int argc, char *argv[])

     QApplication::setAttribute(Qt::AA_EnableHighDpiScaling);

-    SingleApplication a(argc, argv);
+    SingleApplication a(argc, argv, use_multi_instance(argc, argv));

     AppSettings().setupLocalization();

@@ -23,3 +25,15 @@ int main(int argc, char *argv[])

     return a.exec();
 }
+
+const std::string MULTI_INSTANCE_ARG = "--multi-instance";
+
+bool use_multi_instance(int argc, char *argv[])
+{
+    for (int i = 0; i < argc; ++i) {
+        if (argv[i] == MULTI_INSTANCE_ARG) {
+            return true;
+        }
+    }
+    return false;
+}
\ No newline at end of file
Mart-Bogdan commented 1 year ago

I jsut got thought. Would this create problems with helper?

JonMagon commented 1 year ago

I jsut got thought. Would this create problems with helper?

This can cause problems when running simultaneous tests. But the flag idea is a good one.