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

[Benjamin Sperisen]

Hi Josh and Derek,

Thanks for your helpful suggestions! I've just committed a revision
incorporating them, as well as a number of new comments.

On 6/25/07, Derek Atkins <warlord at mit.edu> wrote:
> >> -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?
>
> Or put the state into the parse context and not in the function?  Yes,
> "static" state is BAD.  What happens if you call the importer multiple
> times?  (e.g. you're importing multiple files) The second time you run
> the importer this variable will be set and you're screwed.

Yes, this behavior is indeed ugly. I have tried to find a way to get
the double call to disappear,  since it's the ideal solution, but had
no success. The function is an event handler for the "charmap_changed"
event in the GOCharmapSel object; I don't know why it calls the
handler twice. I think it might be a bug in GOffice, but I'm not sure.
... In any case, I have changed to using a boolean flag in the
GncCsvPreview struct, as Derek suggested.

Benny