2015-07-28 GnuCash IRC logs
00:01:06 *** gjanssens has joined #gnucash
00:01:06 *** ChanServ sets mode: +o gjanssens
00:52:23 *** storyjesse has joined #gnucash
01:51:45 *** storyjesse has quit IRC
02:31:48 *** fell has quit IRC
02:37:48 *** fell has joined #gnucash
02:44:48 *** O01eg has quit IRC
02:58:09 *** cartsoftware has quit IRC
03:21:26 *** cartsoftware has joined #gnucash
03:28:26 *** fabior has joined #gnucash
03:30:22 *** andy has quit IRC
03:30:22 *** cartsoftware1 has joined #gnucash
03:30:55 *** cartsoftware has quit IRC
03:37:10 *** andy has joined #gnucash
04:05:15 *** nomeata has joined #gnucash
04:10:39 *** fabior has quit IRC
06:07:18 *** fabior has joined #gnucash
06:10:39 *** fabior has quit IRC
06:51:31 *** Jimraehl1 has left #gnucash
06:57:19 *** Jimraehl1 has joined #gnucash
07:35:57 *** dgtlmoon has joined #gnucash
07:38:24 *** storyjesse has joined #gnucash
07:40:17 *** himaxx has joined #gnucash
07:42:54 *** himaxx has quit IRC
08:16:25 *** storyjesse has quit IRC
09:13:21 <gjanssens> I'm trying to make sense of the function xaccSplitGetCorrAccountName and friends
09:14:00 <gjanssens> It is supposed to return the account name of "a split on the other side" (according to the comment above it in Split.h)
09:14:31 <gjanssens> That's nice when you are looking at a 2-split transaction
09:15:01 <gjanssens> In case of a multi-split transaction however there is no real "other side" as far as I understand
09:15:45 <gjanssens> Yet the function goes out of its way to try and come up with one by looking for a split with a value of the opposite sign of the base split
09:16:05 <gjanssens> And will return that one if it's the only split of the opposite sign
09:16:31 <gjanssens> Well, the account name for that split that is
09:16:47 <gjanssens> What use case does this serve for a multi-split transaction ?
09:17:32 <gjanssens> Everywhere in the gui "-- Split Transaction --" is displayed in such a case as there is no way gnucash can tell what the "other side" would be
09:18:49 <gjanssens> Worse even, I have identified this peculiar test (multi-split, only one split of opposite sign) to be the root cause of bug 752035
09:18:58 <gjanssens> https://bugzilla.gnome.org/show_bug.cgi?id=752035
09:19:43 <gjanssens> So unless there is a compelling reason to try and find a unique - opposite signed split in a multi-split transaction, I'm tempted to simplify the functions
09:20:06 <gjanssens> For two-split transactions, return the other split (or it's account name or code or whatever is requested)
09:20:38 <gjanssens> For multi-split transactions, I'd simply let it return "-- Split Transaction --" to match what is displayed to the user anyway
09:20:52 <gjanssens> Thoughts ?
09:43:47 <warlord> [ looking at bug ]
09:45:49 <warlord> So... the implementation of that function affects that bug? Have you verified that changing the implementation fixes the bug?
09:46:46 <warlord> It looks (to me) like xaccSplitGetCorrAccountFullName is the only API used from scheme, and it's used in report-utilities.scm.
09:47:27 <warlord> In the C code it looks like the only real use is in csv-transactions-export.c
09:47:47 <warlord> (it's also used internally in Split.c -- I haven't looked at the context.
09:48:38 <warlord> Just looked -- only used in compare functions..
09:49:06 <warlord> So I think that changing that API is *probably* okay...
09:49:25 <warlord> (I'm not sure if the report or csv export is expecting the current behavior)
09:49:41 <gjanssens> I looked through both
09:49:59 <gjanssens> The csv export is using the code to export the other account name
09:50:17 <warlord> Right, but if it's a split transaction what's it supposed to do?
09:50:23 <gjanssens> In that context, a multi-split transaction can't have "one" other account name
09:51:07 <gjanssens> IMO it doesn't make sense to talk of an "other" account name in that context
09:51:39 <gjanssens> We could gracefully avoid this in export by setting "Split Transaction"
09:52:06 <gjanssens> Which is what already happens in most cases except for a very specific combination of splits
09:52:38 <gjanssens> (only one opposite signed split)
09:53:09 <gjanssens> Of the reports only the transaction report was really using this function
09:53:46 <gjanssens> And there it surfaced the bug because the author of that part of the report didn't properly consider how xaccSplitGetCorrAccountFullName works
09:54:14 <gjanssens> So there the indended functioning is exactly what I propose
09:54:52 <gjanssens> I also note that the comments in Split.h suggest these functions were actually created *specifically for* the transaction report
09:55:10 <gjanssens> Only the way currently implemented it doesn't work there
09:55:15 <gjanssens> Irony...
09:56:10 <gjanssens> With my updated function it does work as advertised
09:56:35 <gjanssens> Tested and compared with the old code for unexpected side-effects
09:57:36 <gjanssens> How long does gnucash support multi-split transactions already ?
09:57:48 <warlord> Forever
09:57:51 <gjanssens> I'll note that the buggy function dates from before 1.9.3
09:57:57 <warlord> It did it since before I joined in 1999
09:58:11 <warlord> Oh, I'm sure it's been around since 1.4 or even earlier.
09:58:52 <gjanssens> Ok, then the bug has been there since the inception of this function (2006)
09:59:03 *** mlncn has joined #gnucash
09:59:20 <gjanssens> Anyway, I'll push my change then.
09:59:45 <gjanssens> I have added a test case even to catch future regressions :)
10:00:29 <warlord> Works for me.
10:13:55 *** andy has quit IRC
10:14:36 *** mlncn has quit IRC
10:16:15 *** andy has joined #gnucash
10:20:44 *** mlncn has joined #gnucash
11:17:50 *** mlncn has quit IRC
11:29:54 *** nomeata has quit IRC
11:37:11 *** fabior has joined #gnucash
11:43:17 *** himaxx has joined #gnucash
11:43:20 *** fabior has quit IRC
11:44:33 *** fabior has joined #gnucash
11:44:50 *** himaxx has quit IRC
12:07:40 *** uXus has quit IRC
12:11:40 <jralls> gjanssens: Welcome back! Did you enjoy your trip?
12:14:04 <gjanssens> jralls: yes I did
12:14:25 <gjanssens> Had to get used to the enormeous distances though..
12:14:49 <jralls> Yeah, that bends a lot of people's minds.
12:15:03 <gjanssens> In Belgium I can drive across the whole country in less than 3 hours...
12:15:12 <jralls> You're back just in time. I leave for the UK on monday.
12:15:34 <jralls> And it takes 5 for me to drive to LA. Not that I'd want to...
12:16:09 <gjanssens> Looking forward to your Scottland visit ?
12:16:33 <jralls> Very much. A whole week of first-class piping!
12:16:43 <gjanssens> :)
12:17:03 <gjanssens> And single malt whiskeys if you're into that
12:17:17 <gjanssens> My brother in law is
12:18:09 <jralls> I am, but that's available everywhere. The Brits tax the snot out of it, too, so it's more expensive there than here.
12:18:29 <gjanssens> Wow, better drink it at home then
12:19:21 <warlord> Duty free
12:21:43 <jralls> Heh. The markup in the duty-free shops almost matches the VAT discount. I've yet to find any bargains there.
12:21:55 <warlord> LOL
12:26:49 <jralls> On a separate matter, do either of you understand the reasoning behind the complexity of the file options? It seems to me to be an immense amount of overhead to read and write a few KVPs.
12:31:55 <warlord> The idea (at the time) was to re-use the Option code and allow different modules to supply per-data-file settings.
12:32:09 <warlord> So it was a data-driven UI
12:32:34 <warlord> and using KVP meant we were safe from forward and backward version changes.
12:41:22 <jralls> But that can be done simply by providing a qofbook functions to read and write arbitrary options. I provided those functions in my KVP rewrite, but it takes IIRC 6 intermediate calls between the get function in options.scm and the qofbook function, going scheme->C->scheme->C.
12:43:49 <jralls> Then there's the report options, which use the same top-level code but handle storage differently, writing scheme to ~/.gnucash/saved-reports-2.4.
12:53:04 <gjanssens> I still haven't been able to fully grasp all this option code...
12:53:17 <gjanssens> Is that your next rewrite project ?
12:54:40 <gjanssens> jralls: side question - when I want to write new test code, which is the preferred testing framework currently ?
12:54:54 <gjanssens> The engine in maint uses a mix of two different ones
12:55:09 <jralls> It could be, though it's a bit peripheral to the main goal of replacing QofQuery with SQL.
12:55:33 <gjanssens> And I remember you were discussing wit lmat on mock based tests (or something like that)
12:56:15 <gjanssens> I'm currently working on maint
12:56:57 <gjanssens> Well, currently is more "in between when I have some time"
12:57:09 <jralls> For testing C++ directly I'm using Google test. That's only in master, of course. I'm still using the existing tests for the C interfaces.
12:57:49 <gjanssens> Is it worth adding additional tests on maint still ? Or should I do that only on master ?
12:58:09 <gjanssens> I'm thinking of my initial porting work on import/export
12:58:25 <gjanssens> I figured I should first add some tests before starting to port
12:58:37 <gjanssens> Those tests obviously should test the existing C code then
12:58:47 <jralls> For maint you'd continue to use either the legacy test code or the glib test code, whichever is already used for the module you're working on. If the module isn't tested at all, glib test is a lot more modern in style, so I'd prefer that.
13:00:06 <jralls> Yes, absolutely write tests in maint. Every additional test of existing C code helps ensure that the C++ code doesn't break the C API.
13:00:06 <gjanssens> Ok. And the glib tests in our code are the uTest-xyz files ?
13:01:11 <jralls> Yes. Documentation at https://developer.gnome.org/glib/unstable/glib-Testing.html
13:06:34 <jralls> So for today's bug fix of xaccSplitGetCorrAccountFullName, you'll need to adjust the test in utest-Split.c to reflect the new behavior.
13:07:04 <gjanssens> That's part of the commit. I found that one :)
13:07:21 <gjanssens> However, I'm not really clear on how to start from scratch
13:08:45 <jralls> But if you're writing tests with a view to rewriting new C++ you might consider writing those tests with Google Test, which is a lot faster than glib test. That would have to go in master, of course.
13:10:16 <gjanssens> Obviously
13:10:24 <gjanssens> Still on glib tests for now...
13:10:57 <jralls> Glib test needs a test-foo.c to collect and run the test classes. test-engine.c and test-qof.c are examples; the substitutions should be obvious.
13:11:05 <gjanssens> I'm confused about test_suites vs test_modules
13:11:19 <gjanssens> (It's been a while so I may be mixing up names)
13:12:02 <gjanssens> I'll have to refresh my memory before I can ask directed questions
13:12:14 <gjanssens> I'll come back to it later (hopefully before you have left)
13:14:43 <jralls> A test suite is just a collection of test cases. It's what I meant by test class a moment ago. A test case is a test function. You create the test suite by making a function that calls each test case function. See test_suite_split() at the bottom of utest-Split.c.
13:22:06 <gjanssens> Ok
13:22:27 <gjanssens> Got my initial work checked out again
13:22:38 <gjanssens> I remember now I got confused by your template files
13:23:00 <gjanssens> Test suite is clear now
13:23:10 <gjanssens> But then there is still a test-module vs testmain
13:23:20 <gjanssens> How should I interpret these two ?
13:24:10 <gjanssens> Is a test suite supposed to be subdivided in test-modules which in turn call several functions in several testmain copies ?
13:24:37 <gjanssens> Hmm, probably not
13:25:02 *** ErKa has joined #gnucash
13:25:14 <gjanssens> Both testmain and test-module are meant to be built as main test object (both have a main function)
13:25:31 <gjanssens> So I guess I have to choose one or the other
13:25:45 <gjanssens> jralls: what's the difference between these two ?
13:26:25 <jralls> I'm still here, I was just refreshing my memory. ;-)
13:27:15 <gjanssens> Heh, I didn't intend to be pushy...
13:29:39 <jralls> testmain.c is for the probably rare case that you don't want to use suites. test-module.c is the controller for a collection of suites, the template for test-engine.c. test-suite.c is the template for a test suite like utest-Split.c
13:31:31 *** MechtiIde has joined #gnucash
13:31:32 <gjanssens> Right. I'll start by ignoring testmain.c then.
13:31:55 <gjanssens> Perhaps we should add this small snippet of information somewhere that others can find it as well
13:32:15 *** mlncn has joined #gnucash
13:32:58 <gjanssens> I'll add some comments in both documents (to complement your initial comments there)
13:33:41 <jralls> I'll add it to the README in test-templates along with a pointer to src/test-core/unittest-support.h which has some useful macros that make it a bit easier to build the test-suite function and some functions for dealing with catching error messages.
13:36:01 <gjanssens> Ok. I'll leave it to you then.
13:36:15 *** fabior has quit IRC
13:49:56 *** fell has quit IRC
13:52:34 *** MechtiIde has quit IRC
14:06:52 *** MechtiIde has joined #gnucash
14:11:11 *** himaxx has joined #gnucash
14:12:58 *** himaxx has quit IRC
14:34:51 *** MechtiIde has quit IRC
14:35:19 *** ErKa has quit IRC
14:35:51 *** mlncn has quit IRC
14:46:46 *** mlncn has joined #gnucash
14:48:27 *** watzki82 has joined #gnucash
14:58:08 *** watzki82 has quit IRC
15:33:29 *** gjanssens has quit IRC
17:07:39 *** fell has joined #gnucash
17:47:38 *** rpg has joined #gnucash
18:06:46 *** mlncn has quit IRC
18:08:53 *** mlncn has joined #gnucash
18:48:28 *** rpg_ has joined #gnucash
18:50:45 *** rpg has quit IRC
19:32:35 *** O01eg has joined #gnucash
19:36:13 *** O02eg has joined #gnucash
19:37:38 *** O01eg has quit IRC
19:37:53 *** O02eg has quit IRC
19:38:05 *** O01eg has joined #gnucash
22:31:29 *** mlncn has quit IRC
23:42:24 *** rpg has joined #gnucash
23:43:26 *** rpg has joined #gnucash