2009/2/23 Dominik Vogt <[email protected]>: > On Sun, Feb 22, 2009 at 10:14:28PM +0000, Thomas Adam wrote: >> 2009/2/22 Dominik Vogt <[email protected]>: >> > We should make the print_bindings patch stable first. When we're >> > confident the patch is good, I can make the release on the fly. >> >> Absolutely --- >> is there anything outstanding, other than testing it to >> death? > > Maybe you want to look at the code again after my modifications.
Oh I did, and came to the conclusion that: In libs/charmap.c:charmap_table_to_string() - You replaced what I had originally had as "char *c" with a fixed char array. No problem there. - You limited the matching to specific and incremental (to avoid the fact that in the case of modifiers for instance, "A" was included along with "RIWFST", etc., which could have been misleading. Fine. - I can't say I like the magic numbers in the malloc line: allmods = safemalloc(sizeof(table->value) * 8 + 1); However, I know why they're there -- I might add a comment there if I really feel strongly about it. :) This approach is better in that it reduces memory allocation a little. I don't have a problem with it, and it seems to work well. Your changes to fvwm/bindings.c:print_bindings() aren't as extensive. I like (as trivial as it sounds) the block around the code after the case statement -- that does make sense. The only query I have in print_bindings() is I am well aware I have done nothing with "b->Action2", and whilst it is set in places, I am a bit unsure when and if it is actually ever used -- at least, it hasn't had any noticeable effects when dumping out bindings. Perhaps though it's worth considering -- and I'd be happy to do so if I understood its use. Kindly, -- Thomas Adam
