Hi Jonathan
> Attached is the current version of RadioButton Group Name support.
I only have number of small comments, mostly regarding style:
- The DIALOG_VISIBLE flag in formmetadata.cxx suggests this property
should also be visible also in the Dialog Editor - which isn't
necessary since the "normal" controls as used in the Basic/UNO dialogs
do not support this new property
- looking at the spelling of the other properties in the UI, "Group
name" is probably better / more consistent than "Group Name"
- please adhere to the style used in the files you modify. In
particular, please do
if ( foo )
{
...
instead of
if ( foo ) {
...
This helps ensuring a consistent look of the file, and thus better
readability
- nNumSiblings in ORadioButtonModel::SetSiblingPropsTo should be a
sal_Int32 instead of "int". While int probably has at least 32 bits on
all supported platforms, it's a general rule to not use those types
whose concrete size depends on the platform, as this potentially
causes hassle when doing ports to new platforms.
- Any reason why GetGroupName doesn't return the string, but takes it as
out-parameter?
- you need to add HID_PROP_GROUP_NAME to extensions/util/hidother.src,
else our automation QA will complain later
- please try avoiding TABs, the guidelines for OOo code say TABs are to
be expanded to 4 spaces
- please correct the indenting in
ORadioButtonModel::convertFastPropertyValue, the current version is
unnecessarily difficult to read.
Other than this, the patch looks really good.
Ciao
Frank
--
- Frank Schönheit, Software Engineer [EMAIL PROTECTED] -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Base http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]