mate-desktop / mate-screensaver

MATE screen saver and locker
https://mate-desktop.org
GNU General Public License v2.0
48 stars 40 forks source link

gs-watcher-x11: Migrate from dbus-glib to gdbus #230

Open yetist opened 4 years ago

sc0w commented 4 years ago

the indentations with new style inside old style makes the code unreadable at some points

why not just respect old style?

yetist commented 4 years ago

Someone once told me that the unmodified code should be kept, otherwise it will increase the workload of the review.

To be honest, the coding style often causes me trouble when commit code.

raveit65 commented 4 years ago

Same as in https://github.com/mate-desktop/mate-control-center/pull/579 Why not fixing code-style problems in older PRs first.

yetist commented 4 years ago

I really don't know what to do?

  1. Respect the old style and still use Tab indentation.
  2. Use the new style and indent with 4 spaces, which is what this PR does.
  3. Submit 2 times in the PR, the first time only convert the code style to the new format, and the second time modify the code.
yetist commented 4 years ago

The current code respects the old style and uses Tab for indentation. I think it may pass the review.

sc0w commented 4 years ago

new warnings in the logs:

gs-watcher-x11.c:408:38: warning: Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption
                        g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(u)"))) {
                                                          ^~~~~~~~~~~~~~~~~~~~~~

gs-watcher-x11.c:414:38: warning: Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption
                        g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(s)"))) {
                                                          ^~~~~~~~~~~~~~~~~~~~~~
raveit65 commented 4 years ago

Indents! https://patch-diff.githubusercontent.com/raw/mate-desktop/mate-screensaver/pull/230.diff

sc0w commented 4 years ago

I still see the warnings:

gs-watcher-x11.c:408:40: warning: Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption
            g_variant_is_of_type (parameters, G_VARIANT_TYPE("(u)")))
                                              ^~~~~~~~~~~~~~~~~~~~~

gs-watcher-x11.c:415:40: warning: Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption
            g_variant_is_of_type (parameters, G_VARIANT_TYPE("(s)")))
                                              ^~~~~~~~~~~~~~~~~~~~~
yetist commented 4 years ago

I still see the warnings:

Yes, I noticed it, but I don't know how to fix it, it seems that there is no problem with that. https://github.com/search?q=org%3AGNOME+g_variant_is_of_type&type=Code

raveit65 commented 4 years ago

@yetist Can you please squash all commits in one? Than i can start to test it.

yetist commented 4 years ago

@raveit65

OK, done.

raveit65 commented 4 years ago

offtopic on I am using this commit as patch to test it with a RPM package. When opening the with git format-patch.... generated patch in pluma the tab size wasn't shown correct and i thought there was an indent issue. After resetting the tab size from 8 --> 7 --> 8 px the patch was displayed correct in pluma. I did run 2 times in this issue the last days, so it is reproducible for me. Maybe someone other see this issue with pluma too? offtopic off

raveit65 commented 4 years ago

@yetist Is this really tested?

raveit65 commented 4 years ago

Try killall mate-screensaver && mate-screensaver --debug

raveit65 commented 4 years ago

@yetist Screensaver doesn't start and there is a run time warning.

yetist commented 4 years ago

sorry, please keep this PR open, I will migrate mate-session-manager soon.

fossfreedom commented 3 years ago

Hi - just a quick note since I am looking at budgie-screensaver which has the same issues with the deprecated dbus-glib-1 library.

Think this PR should be marked as draft BTW.

https://github.com/mate-desktop/mate-screensaver/blob/master/configure.ac needs reworking to remove the reliance on the library. Doing this also reveals that https://github.com/mate-desktop/mate-screensaver/blob/master/src/gs-listener-dbus.c will also need a rewrite as well.

lukefromdc commented 1 year ago

Any updates on this?