fvwmorg/fvwm3

More old style color code to remove in favor of only using colorsets in the core.

Closed this issue · 3 comments

The command DefaultColors should have been removed with ForeColor and BackColor. In addition it will no longer make sense for DefaultColorset to be -1 by default, (which would revert it to to using the DefaultColors which should be removed). There are also various parts in the colorset code where the colorset is set to -1 if it is not being used to force it to revert to using the old color system, and this should probably be set to either 0 or the DefaultColorset. For instance in update_window_color_style and update_window_color_hi_style in fvwm/style.c, if SUSE_COLORSET fails, it sets the colorset to be -1, which might also need updated with the change of removing the old method of colors and forcing the use of colorsets, since it has been core for so long now and gives a far more versatile way to configure things.

Also might want to consider the role of DefaultColorset, and what it does. It says it controls miscellaneous windows created by fvwm, and mentions the geometry window, but this can now be configured directly and no longer needs to be set via DefaultColorset. It also goes on to talk about how it isn't used in menus, windows, or icon tiles. Outside of the geometry window, what other windows would use this? Does it make sense to remove this, and just default to colorset 0 when no colorset is found (which makes it indirectly the default colorset?)

Hi @somiaj

Thanks. Can you take a look at ta/gh-748? I think it's doing the correct thing, but would appreciate some extra scrutiny.

@ThomasAdam everything seems to work in my tests as expected, and nothing in the commit stands out.

I did notice that the Menu code also uses both single color settings and colorsets (so there is some duplicated code we could remove there if we want to also make Menu's only use colorsets). Though this would be another issue if we want to consider MenuStyle as part of the core. I do agree with leaving modules alone in this.

I'll merge this for now -- MenuStyle will have to come later.