On Fri, 10 Sep 2010 at 20:51:07 +0400, Alexey I. Froloff wrote:
> This patch highlites appicon of a currently focused window.
Cool. But I have one comment.
> --- a/src/actions.c
> +++ b/src/actions.c
> +#ifdef NEWAPPICON
> + wApplicationDeactivate(oapp);
> +#endif
> + }
> +#ifdef NEWAPPICON
> + wApplicationDeactivate(oapp);
> +#endif
> + }
> +#ifdef NEWAPPICON
> + wApplicationActivate(napp);
> +#endif
> --- a/src/application.c
> +++ b/src/application.c
> +#ifdef NEWAPPICON
> + wApplicationDeactivate(wapp);
> +#endif
> --- a/src/application.h
> +++ b/src/application.h
> +#ifdef NEWAPPICON
> +
> +#define wApplicationActivate(wapp) do { \
> + if (wapp->app_icon) { \
> + wIconSetHighlited(wapp->app_icon->icon, True); \
> + wAppIconPaint(wapp->app_icon);\
> + } \
> + } while(0)
> +
> +#define wApplicationDeactivate(wapp) do { \
> + if (wapp->app_icon) { \
> + wIconSetHighlited(wapp->app_icon->icon, False); \
> + wAppIconPaint(wapp->app_icon);\
> + } \
> + } while(0)
> +
> #endif
>
> +#endif
Wouldn't it be cleaner if you simply #define wApplicationActive() etc to
do nothing in the non-NEWAPPICON case? Something like adding a 'else'
in the above to do
#else
#define wApplicationDeactivate(wapp) do { 0 } while (0)
#define wApplicationActivate(wapp) do { 0 } while (0)
#endif
and then avoid all the #ifdefery in src/{actions.c,application.c} ?
That's the kind of cleanup I want to do in other places of wmaker-crm,
so adding one more case now will give me more trouble in the future.
--
To unsubscribe, send mail to [email protected].
