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

added auto close setting to close window when that last tab is closed #613

Merged
merged 2 commits into from Mar 7, 2024

Conversation

tlemy
Copy link
Contributor

@tlemy tlemy commented Dec 11, 2023

Once enabled this new setting will close the window when closing the last tab.

Here is a picture:
Screenshot from 2023-12-11 13-13-05

Correcting issue #612

The new setting was added to the UI of the preferences.
A condition in the close tab command checks if the number of tabs is 0.
If yes, another condition will check if this setting is enabled before closing the window.
It won't close the whole app if there are multiple windows, but only the targeted window.

@tlemy
Copy link
Contributor Author

tlemy commented Dec 24, 2023

Hi, this is a reminder 😉

@tlemy
Copy link
Contributor Author

tlemy commented Dec 29, 2023

Ping @clefebvre @mtwebster.

@mtwebster
Copy link
Member

Hi, I don't have an issue with the feature, but I'm wondering if everything you're doing is necessary:

  • Add setting to the preferences dialog - fine
  • Add check when closing a tab to see if we're the last window - fine

But adding a listener to xed-settings.c, and applying the value to gsettings for each window isn't needed.

These settings are application-wide, and the setting you're adding is controlled entirely by g_settings_bind() - the GET/SET flags mean if you flip the switch, the setting is automatically updated, and vice-versa.

It's also basically a passive setting - nothing needs to be done if the value changes, it's just checked any time a tab is closed. The xed-settings.c and xed-window.c and xed-window.h changes aren't necessary.

@mtwebster mtwebster added the Blocked Needs rebase, changes, or discussion label Jan 31, 2024
@tlemy
Copy link
Contributor Author

tlemy commented Feb 1, 2024

@mtwebster removed the unnecessary changes

@mtwebster mtwebster removed the Blocked Needs rebase, changes, or discussion label Mar 7, 2024
@mtwebster mtwebster merged commit 2168e89 into linuxmint:master Mar 7, 2024
2 checks passed
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 this pull request may close these issues.

None yet

2 participants