AUDIT: r16976 - gnucash/trunk - QIF import: Show the druid's documentation pages by default. Previously these pages were hidden by default.

[Charles Day]

On Thu, Feb 28, 2008 at 3:16 PM, Andreas Köhler <andi5.py at gmx.net> wrote:

> Hi Charles,
>
> Am Donnerstag, den 28.02.2008, 17:50 -0500 schrieb Charles Day:
> > Author: cedayiv
> > Date: 2008-02-28 17:50:39 -0500 (Thu, 28 Feb 2008)
> > New Revision: 16976
> > Trac: http://svn.gnucash.org/trac/changeset/16976
> >
> > Added:
> >    gnucash/trunk/src/import-export/qif-import/schemas/
> >    gnucash/trunk/src/import-export/qif-import/schemas/Makefile.am
> >
>  gnucash/trunk/src/import-export/qif-import/schemas/apps_gnucash_import_qif.schemas.in
> > Modified:
> >    gnucash/trunk/configure.in
> >    gnucash/trunk/src/import-export/qif-import/Makefile.am
> >    gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
> > Log:
> > QIF import: Show the druid's documentation pages by default. Previously
> these pages were hidden by default.
> > BP
>
> I just wonder, whether it is really necessary to backport this one?  Did
> anyone bother before, is there a bug about it?  I know that other
> commits were chosen as well, but e.g. white-space fixes greatly ease
> later merges.  But, hey :-D
>

I feel it is a bug, yes, as Ian stated that those helpful "documentation"
pages were not intended to be hidden by default. This could be filed as a
Bugzilla bug, though Derek, Ian and I happened to work through it on the
mailing list instead.


> > Modified: gnucash/trunk/configure.in
> > ===================================================================
> > --- gnucash/trunk/configure.in        2008-02-28 22:09:25 UTC (rev
> 16975)
> > +++ gnucash/trunk/configure.in        2008-02-28 22:50:39 UTC (rev
> 16976)
> > @@ -1499,6 +1499,7 @@
> >            src/import-export/qif-import/Makefile
> >            src/import-export/qif/Makefile
> >            src/import-export/qif/test/Makefile
> > +          src/import-export/qif-import/schemas/Makefile
> >            src/import-export/qif-import/test/Makefile
> >            src/import-export/qif-io-core/Makefile
> >            src/import-export/qif-io-core/test/Makefile
> >
> > Modified: gnucash/trunk/src/import-export/qif-import/Makefile.am
> > ===================================================================
> > --- gnucash/trunk/src/import-export/qif-import/Makefile.am    2008-02-28
> 22:09:25 UTC (rev 16975)
> > +++ gnucash/trunk/src/import-export/qif-import/Makefile.am    2008-02-28
> 22:50:39 UTC (rev 16976)
> > @@ -1,4 +1,4 @@
> > -SUBDIRS = . test
> > +SUBDIRS = . test schemas
> >
> >  pkglib_LTLIBRARIES=libgncmod-qif-import.la
> >
> >
> > Modified: gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
> > ===================================================================
> > --- gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
> 2008-02-28 22:09:25 UTC (rev 16975)
> > +++ gnucash/trunk/src/import-export/qif-import/druid-qif-import.c
> 2008-02-28 22:50:39 UTC (rev 16976)
> > @@ -2057,6 +2057,7 @@
> >
> >    QIFImportWindow * retval;
> >    GladeXML        * xml;
> > +  GError * err = NULL;
> >    SCM  load_map_prefs;
> >    SCM  mapping_info;
> >    SCM  create_ticker_map;
> > @@ -2209,9 +2210,20 @@
> >    retval->doc_pages        = NULL;
> >    retval->commodity_pages = NULL;
> >
> > +  /* Get the user's preference for showing documentation pages. */
> >    retval->show_doc_pages =
> > -    gnc_gconf_get_bool("dialogs/import/qif", "show_doc", NULL);
> > +    gnc_gconf_get_bool("dialogs/import/qif", "show_doc", &err);
> > +  if (err != NULL) {
> > +    /* The setting can't be found. */
>
> That is not how I understand gconf_client_get_bool(), the function
> underlying gnc_gconf_get_bool().  To me it seems that err will not be
> set if the key is simply unset, but rather if something else fails, like
> if a non-boolean value is stored at the key.  This is also regardless of
> the existence of a schema for the key.
>

You're right. I figured that out during testing. So yes, the comment is a
little misleading, but the code itself seems OK to me.


> > +    printf("QIF import: gnc_gconf_get_bool error: %s\n", err->message);
>
> Please do not use printfs, but rather one of
> g_{debug,message,warning,critical,error}(), as described in
> lib/libqof/qof/qoflog.h.
>

Printf (or "display" in Scheme) is used throughout the QIF importer for
error messages, including bug detection. I simply followed along. Perhaps
it's time to go through all the QIF importer C code to do error reporting
consistently and correctly.

Cheers,
Charles

> +    g_error_free(err);
> >
> > +    /* Show documentation pages by default. */
> > +    printf("QIF import: Couldn't get show_doc setting from gconf.\n");
> > +    printf("QIF import: Documentation pages will be shown by
> default.\n");
> > +    retval->show_doc_pages = TRUE;
> > +  }
> > +
> >    for(i=0; i < NUM_PRE_PAGES; i++) {
> >      retval->pre_comm_pages =
> >        g_list_append(retval->pre_comm_pages,
> --8<---
>
> Ciao,
> -- andi5
>
>
> _______________________________________________
> gnucash-devel mailing list
> gnucash-devel at gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
>