mate-desktop / mate-session-manager

MATE session manager
https://mate-desktop.org
GNU General Public License v2.0
28 stars 35 forks source link

capplet: use a function to set the visibility of treeview rows #274

Closed rbuj closed 3 years ago

rbuj commented 3 years ago

closes #260

raveit65 commented 3 years ago

For what is this ?

rbuj commented 3 years ago

It improves the code comprehension and it removes unused arguments.

The _app_added and _app_removed methods are used for updating the applications listed on the treeview when there is a change in the manager. The update_tree_view method adds or removes the applications which are not listed by default on the treeview (see https://github.com/mate-desktop/mate-session-manager/commit/16fe21a84ea333db67e9f0509d619f8d3fee1443)

raveit65 commented 3 years ago

Does it fix this report? https://github.com/mate-desktop/mate-session-manager/issues/156

rbuj commented 3 years ago

No it isn't. It's a frontend bugfix.

156 may be related to the backend, ie manager. I haven't reviewed it yet:

raveit65 commented 3 years ago

I tested all 3 PRs from you and my PR before you updated this PR, and the result was that the fix for https://github.com/mate-desktop/mate-session-manager/issues/156 doesn't work proper any more. With current changes we have a conflict with my PR and one of yours.

For me the important thing is to have a proper fix for https://github.com/mate-desktop/mate-session-manager/issues/156 which was done with https://github.com/mate-desktop/mate-session-manager/pull/272

rbuj commented 3 years ago

272 adds a regression as it shows all applications after launching the application because it removes update_tree_view call after calling populate_model. It neither disables the launch of system applications on startup.

$ ps ax | grep screensaver
 197417 ?        Ssl    0:00 /usr/bin/mate-screensaver --no-daemon

Captura de pantalla a 2021-04-18 17-58-11

raveit65 commented 3 years ago

Mate-screensaver is always shown in dialog since the begining of MATE desktop. This isn't wrong! Edit: When show-hidden is enabled we see all applications in /etc/XDG/autostart/ Still wondering why you think this is wrong......

rbuj commented 3 years ago

@raveit65 I mean the "show hidden" checkbox. Captura de pantalla a 2021-04-18 18-27-12

$ grep NoDisplay /etc/xdg/autostart/mate-screensaver.desktop
NoDisplay=true
raveit65 commented 3 years ago

When i only use https://github.com/mate-desktop/mate-session-manager/pull/272 i can disable hidden application and i can show hidden applictions in my test. And with both values i can disable/enable a start of an application. I don't get what is wrong in real testing. Don't understand me wrong.... If you really did test my PR and think this is wrong than please provide a better solution to fix https://github.com/mate-desktop/mate-session-manager/issues/156. I am not against an improvement.

But at the moment i see only 3 new PRs but enable/disable applications in autostart isn't fixed.

rbuj commented 3 years ago

Try to disable Bluetooth OBEX Agent, which is not shown if the checkbox "show hidden" is not checked:

$ grep NoDisplay /etc/xdg/autostart/blueberry-obex-agent.desktop
NoDisplay=true

After disabling it, it should create the file below:

$ diff .config/autostart/blueberry-obex-agent.desktop /etc/xdg/autostart/blueberry-obex-agent.desktop
13d12
< X-MATE-Autostart-enabled=false

Log out/in, check blueberry-obex-agent:

$ grep Exec /etc/xdg/autostart/blueberry-obex-agent.desktop
Exec=/usr/lib/blueberry/blueberry-obex-agent.py
$ ps ax | grep blueberry-obex-agent
rbuj commented 3 years ago

Ok, I think the bug you're referring to is that it doesn't save the change after you clicked the checkbox, and it's only saved when you clicked on another item of list.

rbuj commented 3 years ago

on_startup_enabled_toggled is working properly.

raveit65 commented 3 years ago

The bug is that enable/disable a service with the checkbox in main GUI doesn't change the settings, a workaround is to open the preferences of a service and save the status of a service there. The issue only exists if enable-hidden checkbox is checked. Described here: https://github.com/mate-desktop/mate-session-manager/issues/156#issuecomment-808731386 https://github.com/mate-desktop/mate-session-manager/issues/156#issuecomment-808893461

raveit65 commented 3 years ago

and it's only saved when you clicked on another item of list.

Looks like you found another workaround :)

rbuj commented 3 years ago

and it's only saved when you clicked on another item of list.

Looks like you found another workaround :)

It's because of GSP_APP_SAVE_DELAY. There is a delay of two seconds before saving the file.

Anyway the files are generated in $HOME/.config/autostart folder when there is any difference with the original file, and they are deleted otherwise.

raveit65 commented 3 years ago

I can confirm that this PR fixes https://github.com/mate-desktop/mate-session-manager/issues/156 and other open variants of the same issue. Thank you Which part fixes enable/start services? We need to fix the issue for 1.24 branch too.

raveit65 commented 3 years ago

@rbuj Simply cherry-picking doesn't work for stable branch. Can this be done for 1.24 branch, please?

rbuj commented 3 years ago

ok, wip

rbuj commented 3 years ago

PR #279