... but reading the patch itself, looks good. Couple stylistic notes: new routines should be defined 'static' in addition to being declared static. Also it's best if the code can be organized so that internal routines are defined before use, then you can omit the forward declarations altogether.
Substantive notes: since _NET_WM_WINDOW_TYPE is set (only) by the application, GetNetWmType() doesn't need to query the window property; it would work just as well to add a new Tcl_Obj * member to struct WmInfo.
The earlier fix for Bug#2864685 should be reworked to use this feature; _NET_WM_WINDOW_TYPE is being set in multiple code paths now, which is not good.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
(pat notes on the chat: patch was made against Tk 8.5. unix/tkUnixWm.c has suffered from constipation and emacs auto-formatting between 8.5 and current HEAD, which is why the patch wouldn't apply there. Suggested course of action: finish the patch on HEAD, then copy final tkUnixWm.c wholesale to core-8-5-branch.)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Updated the patch to conform to the TIP as currently written and to accommodate Joe's comments on the style and using a single function for setting the window type. This patch also makes use of this feature in the Tk library scripts and the demo.
Still to be applied against core-8-5-branch.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In order to form an opinion about this patch for TIP #359, a few minor remarks. The functions SetNetWmType and GetNetWmType lack a "static" keyword in its implementation. The function TkSetTransientFor should either be static or included in some (X11-specific) stub table.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Patch
Patch fails to apply -- neither 'patch -p1' nor 'git apply' seem to want to take it.
What branch was this made against?
... but reading the patch itself, looks good. Couple stylistic notes: new routines should be defined 'static' in addition to being declared static. Also it's best if the code can be organized so that internal routines are defined before use, then you can omit the forward declarations altogether.
Substantive notes: since _NET_WM_WINDOW_TYPE is set (only) by the application, GetNetWmType() doesn't need to query the window property; it would work just as well to add a new Tcl_Obj * member to struct WmInfo.
The earlier fix for Bug#2864685 should be reworked to use this feature; _NET_WM_WINDOW_TYPE is being set in multiple code paths now, which is not good.
(pat notes on the chat: patch was made against Tk 8.5. unix/tkUnixWm.c has suffered from constipation and emacs auto-formatting between 8.5 and current HEAD, which is why the patch wouldn't apply there. Suggested course of action: finish the patch on HEAD, then copy final tkUnixWm.c wholesale to core-8-5-branch.)
Updated the patch to conform to the TIP as currently written and to accommodate Joe's comments on the style and using a single function for setting the window type. This patch also makes use of this feature in the Tk library scripts and the demo.
Still to be applied against core-8-5-branch.
git patch against core-8-5-branch
In order to form an opinion about this patch for TIP #359, a few minor remarks. The functions SetNetWmType and GetNetWmType lack a "static" keyword in its implementation. The function TkSetTransientFor should either be static or included in some (X11-specific) stub table.
[SG]etNetWmType functions have already been fixed. I'll add static to TkSetTransientFor. It is not needed to be exposed.
Applied patch to 8.6 HEAD.
Applied backport to 8.5 branch
You can't close this yet; a backport to 8.4 was promised.