DO NOT REPLY TO THIS MESSAGE. INSTEAD, POST ANY RESPONSES TO THE LINK BELOW.
[STR New] Link: http://www.fltk.org/str.php?L2757 Version: 1.3-feature David, thanks for your continued effort to post patches to improve FLTK. I looked at this one, and there are some points I want to comment on to make it easier for the FLTK team to accept your patches. My comments here are more of a general nature, less specific to this particular patch. (1) Please take a look at FLTK's CMP to see what coding style and conventions we use. Although there are some deviations, this is what the coding style *should* be. Note that I don't say that you didn't do it correctly (especially indenting, tabs, brackets etc.), but just in case you were not aware of it... http://www.fltk.org/cmp.php http://www.fltk.org/cmp.php#CODING_STANDARDS (2) Method names in FLTK use some naming conventions that your patches should also use. Especially the simple set and get methods do always use the same name, with different arguments - so... void enable_item_shortcuts() and void disable_item_shortcuts() should be void item_shortcuts(int) where the int argument is 0 or 1 to disable or enable item shortcuts, resp.. bool using_item_shortcuts() would then be int item_shortcuts() (we're always using int instead of bool for historical reasons, however this may change in the future). (3) It is good that you wrote documentations for your new methods, but it would be even better if you used normal upper case letters where appropriate (e.g. at the beginning of a sentence). (4) I'm not a fan of single-character variable names, particularly if the name is 'l' (lower case L) that can easily be confused with 1 (the digit). Examples from your patch: test_shortcut(item_text(l),... if ((l=item_next(l))==NULL) { ... The latter (4) is my personal opinion, but I believe that the code would be much better maintainable, if such variable names were avoided. (Please don't say "But they are also elsewhere in the code..." - that's true, but I would ask you to avoid this in new code, nevertheless...). Also, please note that most of your RFC's will probably be moved (temporarily ?) to 1.4-feature to prepare for upcoming 1.3.x releases. Depending on the FLTK 3 development progress, they might even be postponed to FLTK 3.0 or later, because we currently have to manage two different branches, and double code maintenance should be avoided as much as possible. Link: http://www.fltk.org/str.php?L2757 Version: 1.3-feature _______________________________________________ fltk-dev mailing list [email protected] http://lists.easysw.com/mailman/listinfo/fltk-dev
