AUDIT: r16976 - gnucash/trunk - QIF import: Show the druid's documentation pages by default. Previously these pages were hidden by default.
Charles Day
cedayiv at gmail.com
Thu Feb 28 18:45:23 EST 2008
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
>
>
More information about the gnucash-devel
mailing list