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]

Reply via email to