Swordfish90 / cool-retro-term

A good looking terminal emulator which mimics the old cathode display...
21.76k stars 836 forks source link

Bug: Cool-retro-term does not heed preserved window size in app settings, falls back to 1024x768 default #824

Open kcbrown opened 7 months ago

kcbrown commented 7 months ago

When you close cool-retro-term, it stores the current window positioning and dimensions in the app settings structure (a sqlite3 database). The intention behind this is to restore those settings at startup so that when you re-open the application, it picks up where it left off as regards size and positioning.

Under Ubuntu 23.04 (and, really, all of the recent versions of Ubuntu I've used this on), this doesn't work.

The reason is that the ordering of object initialization isn't guaranteed by the underlying Qt framework, and the end result is that, at least on the platforms I've used it, the main window initializes before the settings are loaded from the database. This results in the defaults being used.

Here's the fix:

diff --git a/app/qml/main.qml b/app/qml/main.qml
index 8ae0774..9848dbb 100644
--- a/app/qml/main.qml
+++ b/app/qml/main.qml
@@ -26,14 +26,32 @@ import "menus"
 ApplicationWindow {
     id: terminalWindow

-    width: 1024
-    height: 768
+    width: (appSettings.width > 0 ? appSettings.width : 1024)
+    height: (appSettings.height > 0 ? appSettings.height : 768)
+
+    property bool initialized: false

     // Save window properties automatically
-    onXChanged: appSettings.x = x
-    onYChanged: appSettings.y = y
-    onWidthChanged: appSettings.width = width
-    onHeightChanged: appSettings.height = height
+    onXChanged: {
+        if (initialized) {
+            appSettings.x = x
+        }
+    }
+    onYChanged: {
+        if (initialized) {
+            appSettings.y = y
+        }
+    }
+    onWidthChanged: {
+        if (initialized) {
+            appSettings.width = width
+        }
+    }
+    onHeightChanged: {
+        if (initialized) {
+            appSettings.height = height
+        }
+    }

     // Load saved window geometry and show the window
     Component.onCompleted: {
@@ -41,6 +59,7 @@ ApplicationWindow {
         y = appSettings.y
         width = appSettings.width
         height = appSettings.height
+        console.warn("onCompleted called.  Width = ", width, ", height = ", height)

         visible = true
     }
@@ -55,6 +74,19 @@ ApplicationWindow {

     menuBar: qtquickMenuLoader.item

+    Connections {
+        target: appSettings
+
+        function onInitializedSettings() {
+            x = appSettings.x
+            y = appSettings.y
+            width = appSettings.width
+            height = appSettings.height
+            initialized = true
+            console.warn("onInitializedSettings called.  Width = ", width, ", height = ", height)
+        }
+    }
+
     Loader {
         id: qtquickMenuLoader
         active: !appSettings.isMacOS && appSettings.showMenubar
ssrublev commented 6 months ago

Same here with Fedora 39

Inu-Yasha commented 3 months ago

I'm using Fedora 39 as well and came here for a similar reason. I think, a) the settings should have an option on whether to preserve the last time size b) there should be also options to define custom size or launching as maximized c) there should also be command line options for using custom geometry and to launch maximized - there's only one for launching fullscreen according to the man-page. If I'm wrong, the man page is wrong and doesn't list all the options.