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

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

Open
kcbrown opened this issue Nov 18, 2023 · 2 comments

Comments

@kcbrown
Copy link

kcbrown commented Nov 18, 2023

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
@kcbrown kcbrown changed the title Cool-retro-term does not heed preserved window size in app settings, falls back to 1024x768 default Bug: Cool-retro-term does not heed preserved window size in app settings, falls back to 1024x768 default Nov 18, 2023
@ssrublev
Copy link

ssrublev commented Jan 6, 2024

Same here with Fedora 39

@Inu-Yasha
Copy link

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.

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

No branches or pull requests

3 participants