Minor bug fixes

[unixgeek]

I migrated over to q4wine from PlayOnLinux and noticed a few things that were not working properly, so I attempted to fix them. For the terminal fix, an alternative might be to always append /bin/sh -c as the worst case scenario would be an extra sh session (I think). The biggest bug I found was the problem parsing mtab. I even checked another project (udisks I think) and their code was the same as yours and it did not work.

Fixed a bug where /etc/mtab was not being read and resulted in being unable to unmount iso images.
Fixed a bug where the temporary director for generating icons wasn't being deleted. Refactored this to use QDir::tempPath().
Added workaround for st terminal, similar to konsole.
Added a feature to keep the terminal open when running winetricks so users can see the output.

[brezerk]

Hi @unixgeek

Thank you for the pull request:

Fixed a bug where /etc/mtab was not being read and resulted in being unable to unmount iso images.

Haven't been mounting images for a while tbh. However I was able to reproduce the issue. It is bit weird that while`readLine` does not seems to be working as expected. I guess it is related to Qt5. Anyway, I need investigate it a bit.

Fixed a bug where the temporary director for generating icons wasn't being deleted. Refactored this to use QDir::tempPath().

Yeah. /tmp would be preferred. Thanks.

Added workaround for st terminal, similar to konsole.

There are so many terminals this days :) I wonder if we can just append '/bin/sh -c' regrades to terminal name. As you mentioned: the worst case scenario would be an extra sh session

Added a feature to keep the terminal open when running winetricks so users can see the output.

No need to pass -k option (or similar). Looks good. Thanks.

[brezerk]

Okay. So for /etc/mtab it seems like atEnd has been changed in Qt5. The preferred way is to check readLineInto return value.

Following code works for me:

diff --git a/src/q4wine-lib/q4wine-lib.cpp b/src/q4wine-lib/q4wine-lib.cpp
index 74852d5..9b64cd0 100644
--- a/src/q4wine-lib/q4wine-lib.cpp
+++ b/src/q4wine-lib/q4wine-lib.cpp
@@ -743,8 +743,8 @@ QString corelib::getMountedImages(QString cdrom_mount) const{
     QFile file(filename);
     if (file.open(QIODevice::ReadOnly | QIODevice::Text)){
         QTextStream in(&file);
-        while (!in.atEnd()) {
-            QString line = in.readLine();
+        QString line;
+        while (in.readLineInto(&line)) {
 #ifdef DEBUG
             qDebug()<<"corelib::/etc/mtab:line"<<line;
 #endif
@@ -759,8 +759,8 @@ QString corelib::getMountedImages(QString cdrom_mount) const{
                     QFile file(filename);
                     if (file.open(QIODevice::ReadOnly | QIODevice::Text)){
                         QTextStream in(&file);
-                        while (!in.atEnd()) {
-                            QString line = in.readLine();
+                        QString line;
+                        while (in.readLineInto(&line)) {
 #ifdef DEBUG
                             qDebug()<<"corelib::getMountedImages:line"<<line;
 #endif

[unixgeek]

Cool. I probably should have added in my original comments that I barely know C++ and don't know Qt, but I recognize that your mtab parsing code was more optimal than the workaround I came up -- I was just trying to make it work.

[brezerk]

No worries. I barely know C++ and Qt too :D Can you update your PR with suggested code so I merge it in one shot please?

[unixgeek]

I think I got it updated.

[brezerk]

merged. thanks!