r16208 - gnucash/branches/csv-import/src/import-export/csv - Added a bunch of comments to gnc-csv-import.c.

[Josh Sled]

Benjamin Sperisen <lasindi at cvs.gnucash.org> writes:
> Log:
> Added a bunch of comments to gnc-csv-import.c.

> Modified:
> gnucash/branches/csv-import/src/import-export/csv/gnc-csv-import.c

> +/* This struct contains all of the data relevant to the preview dialog
> + * that lets the user configure an import. */

You should take a quick look at the doxygen syntax
<http://www.stack.nl/~dimitri/doxygen/docblocks.html>.  In short, if you make
these "javadoc-style" comments /** ... **/, then they'll show up in the
generated documentation, basically for free.  There are some other tags and
rules that are useful for annotating parts of the documentation strings like
variables and identifiers.

(Note that in terms of doxygen, we use the 'javadoc' (not Qt) form, and '@'
(not '\').)

>  typedef struct
>  {
[...]
> +  GtkDialog* dialog; /* The dialog */

This comment doesn't add anything, and doesn't need to be there.


> +  GtkTreeView* treeview; /* The treeview containing the data */
> +  GtkTreeView* ctreeview; /* The treeview containing the column types */
> +  GtkCheckButton* sep_buttons[SEP_NUM_OF_TYPES]; /* Checkbuttons for common separators */
> +  GtkCheckButton* custom_cbutton; /* The checkbutton for a custom separator */
> +  GtkEntry* custom_entry; /* The entry for custom separators */
> +  gboolean approved; /* This is FALSE until the user clicks "OK". */
>  } GncCsvPreview;

For these, for doxygen, you'll want to say /**< This is FALSE [...] */, I think.


> +/* Event handler for when one of the separator checkbuttons is clicked. */
>  static void sep_button_clicked(GtkCheckButton* widget, GncCsvPreview* preview)
>  {
> +  /* The stock separator charactors */
>    char* sep_chars[] = {" ", "\t", ",", ":", ";", "-"};

Why not just call the variable "stock_sep_chars", and dispense with the
comment?

> +  /* A list of all the separators that have been checked. */
>    GSList* separators = NULL;

Similarly, if the variable is 'checked_separators'...


> +  /* Go through each of the separator buttons. */
>    for(i = 0; i < SEP_NUM_OF_TYPES; i++)
>    {
> +    /* If this button is checked, add the corresponding character to
> +     * the separators list. */
>      if(gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(preview->sep_buttons[i])))
>        separators = g_slist_append(separators, sep_chars[i]);
>    }
> +  /* If the custom button is checked ... */
>    if(gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(preview->custom_cbutton)))
>    {
> +    /* ... get the separator string from the custom entry ... */
>      char* custom_sep = (char*)gtk_entry_get_text(preview->custom_entry);
> -    if(custom_sep[0] != '\0')
> +    if(custom_sep[0] != '\0') /* ... and if it isn't blank add it to the separators list. */

All of these comments describe exactly what the code already says.  They are
no longer signal.


> -static void encoding_selected(GOCharmapSel* selector, const char* enc,
> +/* Event handler for a new encoding being selected. */
> +static void encoding_selected(GOCharmapSel* selector, const char* encoding,
>                                GncCsvPreview* preview)
>  {
> +  /* This gets called twice everytime a new encoding is selected. The
> +   * second call actually passes the correct data; thus, we only do
> +   * something the second time this is called. */
>    static gboolean second_call = FALSE;
> +  /* If this is the second time the function is called ... */
>    if(second_call)

Ick.  This stateful behavior should be avoided if at all possible.  Is there
a way to instead be conditional on the "correctness" of the data?  Or
eliminate the double-call?

-- 
...jsled
http://asynchronous.org/ - a=jsled; b=asynchronous.org; echo ${a}@${b}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
Url : http://lists.gnucash.org/pipermail/gnucash-devel/attachments/20070623/f50c1e0a/attachment.bin