qof_instance_set/get_kvp cannot handle boolean?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

qof_instance_set/get_kvp cannot handle boolean?

Carsten Rinke
Hi,

I try to make use of the "TaxRelated" flag of the accounts by calling
xaccAccountSetTaxRelated (Account.c).

I notice that on master this does not lead to the same result as on maint.

To figure out what is wrong/different on master I simply added some
printouts:

========= snip start ===========================

gboolean
xaccAccountGetTaxRelated (const Account *acc)
{
     GValue v = G_VALUE_INIT;
     g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE);
     qof_instance_get_kvp (QOF_INSTANCE(acc), "tax-related", &v);

if (G_VALUE_HOLDS_BOOLEAN (&v))
printf("G_VALUE_HOLDS_BOOLEAN returns TRUE\n");  <=============
else
printf("G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is
%s\n",G_VALUE_TYPE_NAME(&v));  <=============

if (g_value_get_boolean (&v))
printf("g_value_get_boolean returns TRUE\n");  <=============
else
printf("g_value_get_boolean returns FALSE\n");  <=============

     return G_VALUE_HOLDS_BOOLEAN (&v) ? g_value_get_boolean (&v) : FALSE;
}

void
xaccAccountSetTaxRelated (Account *acc, gboolean tax_related)
{
if (tax_related)
printf("Request to set TaxRelated TRUE\n");  <=============
else
printf("Request to set TaxRelated FALSE\n");  <=============

     GValue v = G_VALUE_INIT;
     g_return_if_fail(GNC_IS_ACCOUNT(acc));

     g_value_init (&v, G_TYPE_BOOLEAN);
     g_value_set_boolean (&v, tax_related);

     xaccAccountBeginEdit(acc);
     qof_instance_set_kvp (QOF_INSTANCE (acc), "tax-related", &v);
     mark_account (acc);
     xaccAccountCommitEdit(acc);

if (xaccAccountGetTaxRelated(acc))
printf("TaxRelated is now TRUE\n");  <=============
else
printf("TaxRelated is now FALSE\n");  <=============
}

=========== snip end =========================

The printout shows

Request to set TaxRelated TRUE
G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is gchararray
g_value_get_boolean returns FALSE
TaxRelated is now FALSE

My suspicion:

in kvp_frame.cpp in function

KvpValue*
kvp_value_from_gvalue (const GValue *gval)

============= snip start ===========
     else if (type == G_TYPE_BOOLEAN)
     {
         auto bval = g_value_get_boolean(gval);
         if (bval)
             val = new KvpValue(g_strdup("true"));
======= snip end =================

the information is lost that the origin is boolean. After this point I
get lost.

What to do? Open a bug report?

Kind regards,
Carsten

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: qof_instance_set/get_kvp cannot handle boolean?

Christian Stimming-4
Am Mittwoch, 21. September 2016, 15:35:36 schrieb Carsten Rinke:
> the information is lost that the origin is boolean. After this point I
> get lost.
>
> What to do? Open a bug report?

I haven't worked with the kvp code in a while. As you're asking what to do,
here's my suggestion: Try to put together some unittest code that exihibit the
unexpected (or: erroneous) behaviour. Once you are able to put this together,
it should become clear whether this is a bug in the kvp code and/or in your
usage of it, possibly caused by erroneous documentation. Either way, once you
have the unittest code that demonstrates the problematic behaviour, you can
either open a bug report or ask here for getting this fixed.

Regards,
Christian
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: qof_instance_set/get_kvp cannot handle boolean?

John Ralls-2
In reply to this post by Carsten Rinke

> On Sep 21, 2016, at 3:35 PM, Carsten Rinke <[hidden email]> wrote:
>
> Hi,
>
> I try to make use of the "TaxRelated" flag of the accounts by calling xaccAccountSetTaxRelated (Account.c).
>
> I notice that on master this does not lead to the same result as on maint.
>
> To figure out what is wrong/different on master I simply added some printouts:
>
> ========= snip start ===========================
>
> gboolean
> xaccAccountGetTaxRelated (const Account *acc)
> {
>    GValue v = G_VALUE_INIT;
>    g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE);
>    qof_instance_get_kvp (QOF_INSTANCE(acc), "tax-related", &v);
>
> if (G_VALUE_HOLDS_BOOLEAN (&v))
> printf("G_VALUE_HOLDS_BOOLEAN returns TRUE\n");  <=============
> else
> printf("G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is %s\n",G_VALUE_TYPE_NAME(&v));  <=============
>
> if (g_value_get_boolean (&v))
> printf("g_value_get_boolean returns TRUE\n");  <=============
> else
> printf("g_value_get_boolean returns FALSE\n");  <=============
>
>    return G_VALUE_HOLDS_BOOLEAN (&v) ? g_value_get_boolean (&v) : FALSE;
> }
>
> void
> xaccAccountSetTaxRelated (Account *acc, gboolean tax_related)
> {
> if (tax_related)
> printf("Request to set TaxRelated TRUE\n");  <=============
> else
> printf("Request to set TaxRelated FALSE\n");  <=============
>
>    GValue v = G_VALUE_INIT;
>    g_return_if_fail(GNC_IS_ACCOUNT(acc));
>
>    g_value_init (&v, G_TYPE_BOOLEAN);
>    g_value_set_boolean (&v, tax_related);
>
>    xaccAccountBeginEdit(acc);
>    qof_instance_set_kvp (QOF_INSTANCE (acc), "tax-related", &v);
>    mark_account (acc);
>    xaccAccountCommitEdit(acc);
>
> if (xaccAccountGetTaxRelated(acc))
> printf("TaxRelated is now TRUE\n");  <=============
> else
> printf("TaxRelated is now FALSE\n");  <=============
> }
>
> =========== snip end =========================
>
> The printout shows
>
> Request to set TaxRelated TRUE
> G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is gchararray
> g_value_get_boolean returns FALSE
> TaxRelated is now FALSE
>
> My suspicion:
>
> in kvp_frame.cpp in function
>
> KvpValue*
> kvp_value_from_gvalue (const GValue *gval)
>
> ============= snip start ===========
>    else if (type == G_TYPE_BOOLEAN)
>    {
>        auto bval = g_value_get_boolean(gval);
>        if (bval)
>            val = new KvpValue(g_strdup("true"));
> ======= snip end =================
>
> the information is lost that the origin is boolean. After this point I get lost.
>
> What to do? Open a bug report?
>

Carsten,

Sorry, I meant to pick this up yesterday, but I was focussed on Geert's PR and then forgot about it.

There isn't a KVP_TYPE_BOOLEAN in maint or master. This is handled in maint by testing for the existence of the slot; if it exists the value is true, if it doesn't the value is false. The actual stored value is either an int, as in the case of Account's tax-related key, or a string "true".

When I made KVP private I tried to standardize on the string version as that seemed to me to be more obvious when looking at a text XML file. It looks like I screwed up at lease xaccAccountGetTaxRelated and perhaps others. I'll look at fixing that right away.

Regards,
John Ralls


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: qof_instance_set/get_kvp cannot handle boolean?

Robert Fewell-2
I noticed this before while doing my find account dialogue, at the start of
pull request 83 there is a patch for this and the account hidden setting.

On 22 September 2016 at 16:38, John Ralls <[hidden email]> wrote:

>
> > On Sep 21, 2016, at 3:35 PM, Carsten Rinke <[hidden email]> wrote:
> >
> > Hi,
> >
> > I try to make use of the "TaxRelated" flag of the accounts by calling
> xaccAccountSetTaxRelated (Account.c).
> >
> > I notice that on master this does not lead to the same result as on
> maint.
> >
> > To figure out what is wrong/different on master I simply added some
> printouts:
> >
> > ========= snip start ===========================
> >
> > gboolean
> > xaccAccountGetTaxRelated (const Account *acc)
> > {
> >    GValue v = G_VALUE_INIT;
> >    g_return_val_if_fail(GNC_IS_ACCOUNT(acc), FALSE);
> >    qof_instance_get_kvp (QOF_INSTANCE(acc), "tax-related", &v);
> >
> > if (G_VALUE_HOLDS_BOOLEAN (&v))
> > printf("G_VALUE_HOLDS_BOOLEAN returns TRUE\n");  <=============
> > else
> > printf("G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is
> %s\n",G_VALUE_TYPE_NAME(&v));  <=============
> >
> > if (g_value_get_boolean (&v))
> > printf("g_value_get_boolean returns TRUE\n");  <=============
> > else
> > printf("g_value_get_boolean returns FALSE\n");  <=============
> >
> >    return G_VALUE_HOLDS_BOOLEAN (&v) ? g_value_get_boolean (&v) : FALSE;
> > }
> >
> > void
> > xaccAccountSetTaxRelated (Account *acc, gboolean tax_related)
> > {
> > if (tax_related)
> > printf("Request to set TaxRelated TRUE\n");  <=============
> > else
> > printf("Request to set TaxRelated FALSE\n");  <=============
> >
> >    GValue v = G_VALUE_INIT;
> >    g_return_if_fail(GNC_IS_ACCOUNT(acc));
> >
> >    g_value_init (&v, G_TYPE_BOOLEAN);
> >    g_value_set_boolean (&v, tax_related);
> >
> >    xaccAccountBeginEdit(acc);
> >    qof_instance_set_kvp (QOF_INSTANCE (acc), "tax-related", &v);
> >    mark_account (acc);
> >    xaccAccountCommitEdit(acc);
> >
> > if (xaccAccountGetTaxRelated(acc))
> > printf("TaxRelated is now TRUE\n");  <=============
> > else
> > printf("TaxRelated is now FALSE\n");  <=============
> > }
> >
> > =========== snip end =========================
> >
> > The printout shows
> >
> > Request to set TaxRelated TRUE
> > G_VALUE_HOLDS_BOOLEAN returns FALSE, actually it is gchararray
> > g_value_get_boolean returns FALSE
> > TaxRelated is now FALSE
> >
> > My suspicion:
> >
> > in kvp_frame.cpp in function
> >
> > KvpValue*
> > kvp_value_from_gvalue (const GValue *gval)
> >
> > ============= snip start ===========
> >    else if (type == G_TYPE_BOOLEAN)
> >    {
> >        auto bval = g_value_get_boolean(gval);
> >        if (bval)
> >            val = new KvpValue(g_strdup("true"));
> > ======= snip end =================
> >
> > the information is lost that the origin is boolean. After this point I
> get lost.
> >
> > What to do? Open a bug report?
> >
>
> Carsten,
>
> Sorry, I meant to pick this up yesterday, but I was focussed on Geert's PR
> and then forgot about it.
>
> There isn't a KVP_TYPE_BOOLEAN in maint or master. This is handled in
> maint by testing for the existence of the slot; if it exists the value is
> true, if it doesn't the value is false. The actual stored value is either
> an int, as in the case of Account's tax-related key, or a string "true".
>
> When I made KVP private I tried to standardize on the string version as
> that seemed to me to be more obvious when looking at a text XML file. It
> looks like I screwed up at lease xaccAccountGetTaxRelated and perhaps
> others. I'll look at fixing that right away.
>
> Regards,
> John Ralls
>
>
> _______________________________________________
> gnucash-devel mailing list
> [hidden email]
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: qof_instance_set/get_kvp cannot handle boolean?

John Ralls-2

> On Sep 22, 2016, at 6:49 PM, Robert Fewell <[hidden email]> wrote:
>
> I noticed this before while doing my find account dialogue, at the start of pull request 83 there is a patch for this and the account hidden setting.  

I've pushed a fix to master. Sorry, Bob, not yours, I decided to extract the guts to a new function and call it instead of copy&pasting.

Regards,
John Ralls


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: qof_instance_set/get_kvp cannot handle boolean?

Carsten Rinke
Hi John,

took a while, but now I checked: the "TaxRelated" flag of the accounts
is working again as expected.

Thanks a lot,
Carsten


On 23.09.2016 18:30, John Ralls wrote:
>> On Sep 22, 2016, at 6:49 PM, Robert Fewell <[hidden email]> wrote:
>>
>> I noticed this before while doing my find account dialogue, at the start of pull request 83 there is a patch for this and the account hidden setting.
> I've pushed a fix to master. Sorry, Bob, not yours, I decided to extract the guts to a new function and call it instead of copy&pasting.
>
> Regards,
> John Ralls
>
>

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel