[GNC-dev] Bug # 796687

[Geert Janssens]

Op dinsdag 8 januari 2019 01:31:58 CET schreef Alex Aycinena:
> Geert,
> 
> On Sun, Jan 6, 2019 at 9:17 AM Alex Aycinena <alex.aycinena at gmail.com>
> 
> wrote:
> > Geert,
> > 
> > On Sun, Jan 6, 2019 at 8:11 AM Geert Janssens <geert.gnucash at kobaltwit.be>
> > 
> > wrote:
> >> On Sunday, January 6, 2019 4:55:09 AM CET Alex Aycinena wrote:
> >> > Geert,
> >> > 
> >> > I have been investigating bug # 796687 (Tax Entity name and type for an
> >> > account won't work under "Tax Reporting Options" in Gnucash 3.2) and it
> >> > seems that your commit 4053f2ca5331b345f04f75022658d254c36bdd15 on Mar
> >> 
> >> 31,
> >> 
> >> > 2018, has caused book string options not to work. I ran gnucash under
> >> 
> >> gdb
> >> 
> >> > and got the following:
> >> > 
> >> > qof_book_set_string_option (book=0xb49af0,
> >> > 
> >> >     opt_name=0x7fffefcd57e9 "book/tax_US/type", opt_val=0x2600640
> >> 
> >> "F1040")
> >> 
> >> >     at
> >> 
> >> /home/gnucash-dev/gitcheckouts/gnucash-acct-maint-latest/libgnucash/engin
> >> e/q>> 
> >> > ofbook.cpp:1163 1163        qof_book_begin_edit(book);
> >> > 
> >> > going into the function. The book, opt_name and opt_val all seem fine
> >> 
> >> but
> >> 
> >> > the KVP is not saved. When I checked-out the commit just prior to
> >> 
> >> yours, it
> >> 
> >> > seems to work fine: the data is saved and retrieved as expected. After
> >> 
> >> your
> >> 
> >> > commit, it seems to stop working.
> >> > 
> >> > I tried to analyze your commit but I'm not sure I understand it.
> >> > Perhaps
> >> > you could help me understand it. The funny thing is, the unit test
> >> 
> >> seems to
> >> 
> >> > pass.
> >> > 
> >> > Can you help me figure out why it is not working?
> >> > 
> >> > Thanks,
> >> > 
> >> > Alex
> >> 
> >> Alex,
> >> 
> >> I'm not at my development machine until Wednesday. I'll get back to this
> >> later
> >> this week.
> >> 
> >> Geert
> > 
> > OK. Thanks,
> > 
> > Alex
> 
> The following change seems to fix it:
> 
> diff --git a/libgnucash/engine/qofbook.cpp b/libgnucash/engine/qofbook.cpp
> index c0cb043ba..fa67f5b65 100644
> --- a/libgnucash/engine/qofbook.cpp
> +++ b/libgnucash/engine/qofbook.cpp
> @@ -1164,9 +1164,9 @@ qof_book_set_string_option(QofBook* book, const char*
> opt_name, const char* opt_
>      auto frame = qof_instance_get_slots(QOF_INSTANCE(book));
>      auto opt_path = opt_name_to_path(opt_name);
>      if (opt_val && (*opt_val != '\0'))
> -        delete frame->set(opt_path, new KvpValue(g_strdup(opt_val)));
> +        delete frame->set_path(opt_path, new KvpValue(g_strdup(opt_val)));
>      else
> -        delete frame->set(opt_path, nullptr);
> +        delete frame->set_path(opt_path, nullptr);
>      qof_instance_set_dirty (QOF_INSTANCE (book));
>      qof_book_commit_edit(book);
>  }
> 
> If you agree with the change, I will commit it.
> 
> Thanks,
> 
> Alex

Alex,

That patch looks right to me. The difference between set and set_path appears 
to be that set_path creates missing frames where set will do nothing in case 
of missing frames. So if for whatever reason (first use, reset,...) a certain 
string option is not set it will not be created using "set". Perhaps "set" 
would better have been named "update" and "set_path" "create_or_update". Oh, 
well.

Good detective work!

Geert