Window prop list & prop del && Focus policy improvement

[Janek Kozicki]

Hi,

http://sawfish.wikia.com/wiki/Focus_policy_improvement
http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del

How do you think - should those patches go in tandem into sawfish?
Anybody can test them?

--
Janek Kozicki                                                         |

[Christopher Bratusek]

Am Dienstag, den 29.07.2008, 11:01 +0200 schrieb Janek Kozicki:

Hi,

http://sawfish.wikia.com/wiki/Focus_policy_improvement
http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del

How do you think - should those patches go in tandem into sawfish?
Anybody can test them?

The Focus-Policy-Improvement patch relies on this one:

http://sawfish.wikia.com/wiki/Negative-property-p

so should not be ignored.

I tested the focus-policy-improvement patch and it seems to fix the issue Teika found.

Haven't used the other two, but applied it and didn't run into problems.

Chris

GNOME/Sawfish rule!

[Daniel M German]

On Tue, Jul 29, 2008 at 2:01 AM, Janek Kozicki <janek_listy@...> wrote:
> Hi,
>
> http://sawfish.wikia.com/wiki/Focus_policy_improvement
> http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del
>


I am not in a position to comment on the first one, but the second is
a feature that I was
looking for the other day.  So yes, please accept.

--dmg

[Andrea Vettorello]

On Tue, Jul 29, 2008 at 11:01 AM, Janek Kozicki <janek_listy@...> wrote:
> Hi,
>
> http://sawfish.wikia.com/wiki/Focus_policy_improvement
> http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del
>
> How do you think - should those patches go in tandem into sawfish?
> Anybody can test them?

I think I'm running a build with "Window prop list & prop del" and
"Negative-property-p" patches applied from few days without noticing
problems, I've only tested the functions once from sawfish-client tho
and I forgot to apply "Focus policy improvement".


--
Andrea

[Janek Kozicki]

Daniel German said:     (by the date of Tue, 29 Jul 2008 10:03:16 -0700)

> On Tue, Jul 29, 2008 at 2:01 AM, Janek Kozicki <janek_listy@...> wrote:
> > Hi,
> >
> > http://sawfish.wikia.com/wiki/Focus_policy_improvement
> > http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del
> >
> I am not in a position to comment on the first one, but the second is
> a feature that I was
> looking for the other day.  So yes, please accept.

would be useful for me if you had placed your vote on wiki :) Thanks
for your reply anyway, as it is indeed helpful.

--
Janek Kozicki                                                         |

[Timo Korvola-2]

Janek Kozicki <janek_listy@...> writes:
> http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del
>
> How do you think - should those patches go in tandem into sawfish?

The function for removing symbol properties is traditionally called
remprop, so this one should be window-remprop.  Also, it should
probably call property handlers as if the property were set to nil.
Apart from the documented problem with keymaps it seems to work (it
appears that property handlers are only used for window keymaps).

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Janek Kozicki]

Timo Korvola said:     (by the date of Mon, 04 Aug 2008 21:40:40 +0300)

> Janek Kozicki <janek_listy@...> writes:
> > http://sawfish.wikia.com/wiki/Window_prop_list_%26_prop_del
> >
> > How do you think - should those patches go in tandem into sawfish?
>
> The function for removing symbol properties is traditionally called
> remprop, so this one should be window-remprop.  Also, it should
> probably call property handlers as if the property were set to nil.
> Apart from the documented problem with keymaps it seems to work (it
> appears that property handlers are only used for window keymaps).

Thanks, I just prepared a release candidate 1.3.4-rc1 with this patch
applied. So I hope that Teika will submit another patch that will
address your concerns. Otherwise it will be removed from 1.3.4-rc1,
because we cannot make 1.3.4 release with a function that has wrong
(deprecated (?)) name.

wikia.com is down now, so I can't update wiki about the release
candidate, but anyway, you can download it from

https://sourceforge.net/project/showfiles.php?group_id=32

--
Janek Kozicki                                                         |

[Timo Korvola-2]

Janek Kozicki <janek_listy@...> writes:
> Thanks, I just prepared a release candidate 1.3.4-rc1 with this patch
> applied. So I hope that Teika will submit another patch that will
> address your concerns.

Actually, I have the renaming patch at hand.  I just committed it to
trunk.  There is still the property handler issue.  I'll try to have a
look at that later.  Anyway, it only affects keymaps and that is now
documented.

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Timo Korvola-2]

Timo Korvola <Timo.Korvola@...> writes:
> There is still the property handler issue.  I'll try to have a
> look at that later.

OK, I fixed that and some other things.  The wiki page is getting kind
of long but everything is in svn.
<URL:http://sawfish.wikia.com/wiki/Window_prop_list_&_prop_del>

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Christopher Bratusek]

Am Mittwoch, den 06.08.2008, 00:26 +0300 schrieb Timo Korvola:

Timo Korvola <Timo.Korvola@...> writes:
> There is still the property handler issue.  I'll try to have a
> look at that later.

OK, I fixed that and some other things.  The wiki page is getting kind
of long but everything is in svn.
<URL:http://sawfish.wikia.com/wiki/Window_prop_list_&_prop_del>

... That patch again breaks building with CFLAGS="-Wall -ansi -pedantic".
Can someone please fix that. I don't want to do that again now.

It's of course low priority, but it would be nice. Thanks

Chris

GNOME/Sawfish rule!

[Janek Kozicki]

Christopher Bratusek said:     (by the date of Wed, 06 Aug 2008 00:02:02 +0200)

> ... That patch again breaks building with CFLAGS="-Wall -ansi
> -pedantic".
> Can someone please fix that. I don't want to do that again now.
>
> It's of course low priority, but it would be nice. Thanks


I think that after it's fixed, we should keep those flags. You did a
lot of work after all, to get rid of all those warnings.

How do you set those flags? Currently I did:


--- configure.in        (revision 4263)
+++ configure.in        (working copy)
@@ -332,7 +332,7 @@
 dnl set explicitly, add some warning options. Turn off implicit-int
 dnl warnings since the X11 includes on Solaris generate a lot of
these if test "x${GCC}" = "xyes" -a "x$CFLAGS" = "x-g -O2"; then
-  CFLAGS="${CFLAGS} -Wall -Wmissing-prototypes -Wno-implicit-int"
+  CFLAGS="${CFLAGS} -Wall -ansi -pedantic"
 fi


With diff above I tried compiling and fixed just this one:


Index: src/windows.c
===================================================================
--- src/windows.c       (revision 4263)
+++ src/windows.c       (working copy)
@@ -80,7 +80,7 @@
 
 static repv gravity_map[StaticGravity+1];
 
-// In sawfish-1.3.3, the only callback used is keymap_prop_change.
+/* In sawfish-1.3.3, the only callback used is keymap_prop_change. */
 struct prop_handler {
     struct prop_handler *next;
     repv prop;
Index: configure.in


Then I got compilation errors in rep.h (included from client.h). So we
could become -pedantic only after rep-gtk is released.

So I won't be able to fix those errors, because I'd need to
backport rep-gtk on my system...

--
Janek Kozicki                                                         |

[Christopher Bratusek]

Am Mittwoch, den 06.08.2008, 15:36 +0200 schrieb Janek Kozicki:

Christopher Bratusek said:     (by the date of Wed, 06 Aug 2008 00:02:02 +0200)

> ... That patch again breaks building with CFLAGS="-Wall -ansi
> -pedantic".
> Can someone please fix that. I don't want to do that again now. 
> 
> It's of course low priority, but it would be nice. Thanks


I think that after it's fixed, we should keep those flags. You did a
lot of work after all, to get rid of all those warnings.

How do you set those flags? Currently I did:

pass them to configure

CFLAGS="-Wall -ansi -pedantic" ./configure --prefix=/usr

--- configure.in        (revision 4263)
+++ configure.in        (working copy)
@@ -332,7 +332,7 @@
 dnl set explicitly, add some warning options. Turn off implicit-int
 dnl warnings since the X11 includes on Solaris generate a lot of
these if test "x${GCC}" = "xyes" -a "x$CFLAGS" = "x-g -O2"; then
-  CFLAGS="${CFLAGS} -Wall -Wmissing-prototypes -Wno-implicit-int"
+  CFLAGS="${CFLAGS} -Wall -ansi -pedantic"
 fi


With diff above I tried compiling and fixed just this one:


Index: src/windows.c
===================================================================
--- src/windows.c       (revision 4263)
+++ src/windows.c       (working copy)
@@ -80,7 +80,7 @@
 
 static repv gravity_map[StaticGravity+1];
 
-// In sawfish-1.3.3, the only callback used is keymap_prop_change. 
+/* In sawfish-1.3.3, the only callback used is keymap_prop_change. */
 struct prop_handler {
     struct prop_handler *next;
     repv prop;
Index: configure.in


Then I got compilation errors in rep.h (included from client.h). So we
could become -pedantic only after rep-gtk is released.

No rep.h is part of librep, not rep-gtk. get the current trunk of librep, which has both headers
(rep.h and rep_subrs.h) fixed, you'll see, that it is still broken.

So I won't be able to fix those errors, because I'd need to
backport rep-gtk on my system...

Chris

GNOME/Sawfish rule!

[Christopher Bratusek]

Am Mittwoch, den 06.08.2008, 15:44 +0200 schrieb Christopher Bratusek:

Am Mittwoch, den 06.08.2008, 15:36 +0200 schrieb Janek Kozicki:

Christopher Bratusek said:     (by the date of Wed, 06 Aug 2008 00:02:02 +0200)

> ... That patch again breaks building with CFLAGS="-Wall -ansi
> -pedantic".
> Can someone please fix that. I don't want to do that again now. 
> 
> It's of course low priority, but it would be nice. Thanks


I think that after it's fixed, we should keep those flags. You did a
lot of work after all, to get rid of all those warnings.

How do you set those flags? Currently I did:

pass them to configure

CFLAGS="-Wall -ansi -pedantic" ./configure --prefix=/usr

--- configure.in        (revision 4263)
+++ configure.in        (working copy)
@@ -332,7 +332,7 @@
 dnl set explicitly, add some warning options. Turn off implicit-int
 dnl warnings since the X11 includes on Solaris generate a lot of
these if test "x${GCC}" = "xyes" -a "x$CFLAGS" = "x-g -O2"; then
-  CFLAGS="${CFLAGS} -Wall -Wmissing-prototypes -Wno-implicit-int"
+  CFLAGS="${CFLAGS} -Wall -ansi -pedantic"
 fi


With diff above I tried compiling and fixed just this one:


Index: src/windows.c
===================================================================
--- src/windows.c       (revision 4263)
+++ src/windows.c       (working copy)
@@ -80,7 +80,7 @@
 
 static repv gravity_map[StaticGravity+1];
 
-// In sawfish-1.3.3, the only callback used is keymap_prop_change. 
+/* In sawfish-1.3.3, the only callback used is keymap_prop_change. */
 struct prop_handler {
     struct prop_handler *next;
     repv prop;
Index: configure.in


Then I got compilation errors in rep.h (included from client.h). So we
could become -pedantic only after rep-gtk is released.

No rep.h is part of librep, not rep-gtk. get the current trunk of librep, which has both headers
(rep.h and rep_subrs.h) fixed, you'll see, that it is still broken.

So I won't be able to fix those errors, because I'd need to
backport rep-gtk on my system...

Chris

GNOME/Sawfish rule!

fixed. patch waiting for review: http://sawfish.wikia.com/wiki/Another_cleanup

Chris

GNOME/Sawfish rule!

[Timo Korvola-2]

Christopher Bratusek <nano-master@...> writes:
> ... That patch again breaks building with CFLAGS="-Wall -ansi
> -pedantic".

It does?  Which particular version of Sawfish were you able to build
with those flags?  Which versions of Librep and GCC were you using?
How did the patch break it?

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Christopher Bratusek]

Am Freitag, den 08.08.2008, 16:23 +0300 schrieb Timo Korvola:

Christopher Bratusek <nano-master@...> writes:
> ... That patch again breaks building with CFLAGS="-Wall -ansi
> -pedantic".

It does? 

Yes.

 Which particular version of Sawfish were you able to build
with those flags?

Trunk (after my "Major C-Part Clean-Up" patch has been commited)

  Which versions of Librep

Trunk. (I made changes to it)

and GCC were you using?

4.3.1

How did the patch break it?

around line 80. There's one // but that's c++ style. for c it has to be /* */

and the "static struct prop_handler *prop_handlers;" has to be placed before prop_handler{}

the patch I posted fixes both.

Chris

GNOME/Sawfish rule!

[Timo Korvola-2]

Christopher Bratusek <nano-master@...> writes:
> Trunk (after my "Major C-Part Clean-Up" patch has been commited)

As your cleanup patch went in after Teika's patch, it seems that no
committed revision builds with -pedantic -ansi.

> Trunk. (I made changes to it)

Indeed, quite a few it seems.  At the time I was testing librep trunk
did not build with -pedantic -ansi.  By the way, if you want to clean
up the code, you should work toward reducing void * abuse.  void *
should only be used when the pointed type cannot be known at compile
time.  Trying to placate the compiler may lead to the wrong direction
here.

> around line 80. There's one // but that's c++ style. for c it has to
> be /* */

Grepping for // I see it is not the only one, but apparently dlmalloc.c
hasn't been used since 2002.

> and the "static struct prop_handler *prop_handlers;" has to be placed
> before prop_handler{}

It bothers me that I don't understand why.  Otherwise I'd say commit
it.

--
        Timo Korvola <URL:http://www.iki.fi/tkorvola>

[Christopher Bratusek]

Am Samstag, den 09.08.2008, 11:54 +0300 schrieb Timo Korvola:

Christopher Bratusek <nano-master@...> writes:
> Trunk (after my "Major C-Part Clean-Up" patch has been commited)

As your cleanup patch went in after Teika's patch, it seems that no
committed revision builds with -pedantic -ansi.

maybe.

> Trunk. (I made changes to it)

Indeed, quite a few it seems.  At the time I was testing librep trunk
did not build with -pedantic -ansi.  

I know. but the headers are cleaned-up enough, so that sawfish does build with -pedantic -ansi with librep from trunk.

By the way, if you want to clean
up the code, you should work toward reducing void * abuse.  void *
should only be used when the pointed type cannot be known at compile
time.  Trying to placate the compiler may lead to the wrong direction
here.

> around line 80. There's one // but that's c++ style. for c it has to
> be /* */

Grepping for // I see it is not the only one, but apparently dlmalloc.c
hasn't been used since 2002.

Of course, but that's a librep file, I was speaking about sawfish - the clean-up of librep will take much longer than expected.

> and the "static struct prop_handler *prop_handlers;" has to be placed
> before prop_handler{}

It bothers me that I don't understand why.  Otherwise I'd say commit
it.

I also don't know why, sorry.

Chris

GNOME/Sawfish rule!