Re: vincent - r31318 - abiword/branches/gsoc2012dialogs/src/wp/ap/win

Hi Vincent,
All the best for your first commit.

A few suggestions.

On Mon, May 28, 2012 at 10:03 PM, <cvs@abisource.com> wrote:
>
> Author: vincent
> Date: 2012-05-28 18:33:08 +0200 (Mon, 28 May 2012)
> New Revision: 31318
>
> Modified:
>   abiword/branches/gsoc2012dialogs/src/wp/ap/win/ap_Win32Dialog_FormatFrame.cpp
> Log:
> Frame format setting dialog can't remember user's thickness setting.

Hope you are updating the relevant bugzilla ticket/issue, when you
have fixed an issue. Mentioning the issue id (if any) in the commit
log will be helpful as well.

>
> Modified: abiword/branches/gsoc2012dialogs/src/wp/ap/win/ap_Win32Dialog_FormatFrame.cpp
> ===================================================================
> --- abiword/branches/gsoc2012dialogs/src/wp/ap/win/ap_Win32Dialog_FormatFrame.cpp       2012-05-28 16:30:30 UTC (rev 31317)
> +++ abiword/branches/gsoc2012dialogs/src/wp/ap/win/ap_Win32Dialog_FormatFrame.cpp       2012-05-28 16:33:08 UTC (rev 31318)
> @@ -211,10 +211,16 @@
>
>        /* Combo Values for Thickness */
>
> -       for(i=0; i < FORMAT_FRAME_NUMTHICKNESS ;i++)
> +    // current select index in AP_RID_DIALOG_FORMATFRAME_COMBO_THICKNESS

Try to adhere to the AbiWord coding styles. This will make it easy,
when we try to merge your branch to the trunk.

In AbiWord trunk, "docs" folder has AbiSourceCodeGuidelines.abw and
other coding guidelines. They are pretty much outdated, but still are
a good read.

> +       int iCurrentIndex = 0 ;
> +       //most time, all the thickness is same for top,right,bottom,left
> +       UT_UTF8String currentTickness = getBorderThicknessRight();
> +       for(i=0; i < FORMAT_FRAME_NUMTHICKNESS ;i++){

Follow the C++ style in coding. For example, bracket appears in a new
line, instead of the same line of expression, unlike the commonly used
coding style of java.

By going through the code of the same class (or others), you will be
able to find this.

> +               if( currentTickness == sThickness[i]) iCurrentIndex=i;
>                addItemToCombo (AP_RID_DIALOG_FORMATFRAME_COMBO_THICKNESS, sThickness[i]);

Take care with indentation (We use 1 tab which is equal to 4 spaces).
That will increase the readability.

Keep committing regularly. Commits are cheap. Smaller and frequent
commits will help you track your own progress, effectively.

Since we have passed the first week of coding, weekly updates would be
appropriate. You can send a weekly summary of your commits,
discussions, and implementations to this list. Alternatively, you can
post them to your personal blog instead, and share the link with us
(via irc or this list) - whichever you find comfortable for you.

Thank you.
Regards,
Pradeeban.

> +       }
>
> -       selectComboItem (AP_RID_DIALOG_FORMATFRAME_COMBO_THICKNESS, 0);
> +       selectComboItem (AP_RID_DIALOG_FORMATFRAME_COMBO_THICKNESS, iCurrentIndex);
>
>        centerDialog();
>        return 1;
>
> -----------------------------------------------
> To unsubscribe from this list, send a message to
> abisource-cvs-commit-request@abisource.com with the word
> unsubscribe in the message body.

-- 
Kathiravelu Pradeeban.
Blog: [Llovizna] http://kkpradeeban.blogspot.com/