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

Removed display of broken option #572

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Removed display of broken option #572

wants to merge 1 commit into from

Conversation

stefonarch
Copy link
Member

Maybe there is more to clean up after.

@stefonarch stefonarch marked this pull request as draft January 25, 2025 19:34
@stefonarch
Copy link
Member Author

stefonarch commented Jan 25, 2025

I don't understand why but running it it activates screensaver screenlocker also if unchecked at suspend.

@tsujan
Copy link
Member

tsujan commented Jan 26, 2025

I didn't understand what you meant. What's unchecked? Which screensaver? Where?

@stefonarch
Copy link
Member Author

I compiled that and forgot about after checking help and manpage, when I switched to Hyprland the screenlocker was activated on suspend without that this option was checked. Tested labwc and found the same, reinstalled lxqt-session master and all fine again.

@tsujan
Copy link
Member

tsujan commented Jan 26, 2025

Oh, you changed the code too; I didn't see that. With the removal of app.setConfigName(parser.value(config_opt)); the method SessionApplication::setConfigName isn't called anymore, and so, session.conf isn't read. You should replace that line with:

    app.setConfigName();

However, I think it's better to fix that option, instead of removing it. Sadly, I don't remember what its problem was.

@tsujan
Copy link
Member

tsujan commented Jan 26, 2025

Now I remember at least one of the problems: other LXQt config apps assume that the config file is session.conf.

If we want to remove this unused option, we should also change the code a little, because SessionApplication::setConfigName will have no job.

@stefonarch
Copy link
Member Author

That's the reason. I remember I got an compile error about that line.
I don't know if there would be an easy way to tell all other apps to read an alternate config file (a value in session.conf session-config-file= ?) and it would need code changes in all of them afaik. Nobody will edit startlxqt, it could be eventually used in compositor autostart settings but I see no benefit to it.

@tsujan
Copy link
Member

tsujan commented Jan 26, 2025

if there would be an easy way to tell all other apps to read an alternate config file…

There isn't :(

A suggestion: Keeping the option but adding a warning that it isn't compatible with some LXQt configuration tools.

@stefonarch
Copy link
Member Author

Why keeping an non-working option...

@tsujan
Copy link
Member

tsujan commented Jan 26, 2025

In an imaginary world, someone may want to use LXQt Session with that option and without LXQt config tools.

@stefonarch
Copy link
Member Author

With a probability hard al limit del measurable yes.

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.

2 participants