2021-10-17 GnuCash IRC logs

01:15:08 *** frakturfreak2 has quit IRC
01:29:35 *** Mechtilde has quit IRC
01:29:42 *** frakturfreak2 has joined #gnucash
01:46:09 *** Mechtilde has joined #gnucash
01:59:34 *** fell has quit IRC
02:00:51 *** fell has joined #gnucash
02:00:52 *** ChanServ sets mode: +o fell
02:45:32 <byteit101> Does anyone know of a public or "sample" gnucash file? I'm looking to see if there exists a publicly-releasable gnucash file for automated testing. Ideally it would have at least 3 months of plausible transactions, investments, etc
03:53:04 *** kcin has joined #gnucash
04:00:15 *** CDB-Man_ has quit IRC
04:00:17 *** CDB-Man has joined #gnucash
04:00:18 *** ChanServ sets mode: +v CDB-Man
04:27:58 *** miklcct has quit IRC
04:28:05 *** miklcct has joined #gnucash
04:28:05 *** ChanServ sets mode: +v miklcct
04:57:43 *** kcin has quit IRC
05:05:06 *** Aussie_matt has joined #gnucash
05:24:24 *** gjanssens has joined #gnucash
05:24:25 *** ChanServ sets mode: +o gjanssens
05:56:19 *** bertbob has quit IRC
05:58:17 *** bertbob has joined #gnucash
05:58:18 *** ChanServ sets mode: +v bertbob
06:02:12 *** ChanServ sets mode: +v reactormonk[m]
06:02:22 *** User has joined #gnucash
07:02:29 *** gamontecarlo has quit IRC
07:05:37 *** gamontecarlo has joined #gnucash
07:07:29 *** kcin has joined #gnucash
07:21:19 *** mikee has quit IRC
07:21:32 *** alt_mikee has joined #gnucash
07:29:21 *** Aussie_matt has quit IRC
07:55:34 *** Hamaryns has joined #gnucash
07:55:35 *** ChanServ sets mode: +v Hamaryns
08:37:35 *** Hamaryns has quit IRC
08:40:05 *** David has quit IRC
08:40:24 *** David has joined #gnucash
08:54:33 *** storyjesse has joined #gnucash
09:34:46 <warlord> byteit101, there are some small sample files in the source tree, but no, I do not know of any *real* 3-months-of-plausible-date sample file that could be published. You could certainly make one (over three months) by keeping a shadow-version of your own books and mirroring the transactions you make.
09:37:04 *** jervin has joined #gnucash
09:40:22 *** jervin has quit IRC
09:40:31 *** fell has quit IRC
09:40:50 *** fell has joined #gnucash
09:40:51 *** ChanServ sets mode: +o fell
10:28:47 *** storyjesse has quit IRC
10:45:32 *** sbluhm has joined #gnucash
10:56:49 *** sbluhm has quit IRC
11:10:25 <fell> gjanssens: Testiing gnome41 in our flatpak, I see on startup: F: Ignoring D-Conf migrate-path setting /org/gnucash/GnuCash
11:10:55 <fell> leaving most users clueless, I assume.
11:54:16 <gjanssens> fell: do you also get that error with gnome40?
11:57:20 <gjanssens> Oh, it may well be that flatpak is extremely picky and wants a trailing slash in the path. Can you try to add one in the manifest file ?
12:06:14 *** Mechtilde has quit IRC
12:07:33 *** Mechtilde has joined #gnucash
12:28:17 *** bertbob has quit IRC
12:32:47 *** kcin has quit IRC
12:43:00 *** bertbob has joined #gnucash
12:43:00 *** ChanServ sets mode: +v bertbob
12:55:57 *** kcin has joined #gnucash
12:57:28 <fell> gjanssens: rignt, the slash was missing.
13:26:00 *** sbluhm has joined #gnucash
13:33:56 <jralls> gjanssens, I was running the debugger on CSV imports yesterday while figuring out https://bugs.gnucash.org/show_bug.cgi?id=798313. It calls tokenize 7 times. That seems excessive, the tokens don't change, just their interpretation. ISTM tokenize should be split up into 3 functions, tokenize, clear_header_designations, and process_tokens.
13:34:15 *** sbluhm has quit IRC
13:35:57 *** sbluhm has joined #gnucash
13:37:31 *** tomk_dk has joined #gnucash
13:51:43 *** sbluhm has joined #gnucash
13:58:28 *** sbluhm has quit IRC
14:00:46 *** sbluhm has joined #gnucash
14:04:31 *** sbluhm has quit IRC
14:22:04 <gjanssens> jralls: it's been a while since I wrote that code...
14:23:26 <gjanssens> What exactly do these three functions mean to you ? From my interpretation "tokenize" only does that: split each line in the correct number of fields based on given parameters.
14:23:35 <gjanssens> All the rest is no longer part of tokenizing.
14:24:48 <gjanssens> If it's called 7 times I suspect there's some sloppy setup code that calls this function earlier and more often than it should
14:25:05 <jralls> That's my view too, but it's not what's written. https://github.com/Gnucash/gnucash/blob/fbf828476f4c84a9978390e9192ee52878c99eab/gnucash/import-export/csv-imp/gnc-import-tx.cpp#L424
14:26:39 <gjanssens> Ok, I was in a different context. I thought you were referring to the tokenizer class while you really refer to the GcnImport's tokenize function.
14:27:35 <jralls> Right, though the tokenize function calls the tokenizer every time.
14:28:39 <gjanssens> Indeed. Shouldn't it ?
14:29:17 <gjanssens> I thought I had written the code such that tokenize is only called when an event happens that invalidates the currently tokenized data.
14:29:35 <jralls> If that was all GncTxImport::Tokenize did, yes. But it also parses the tokens into scratch transactions and splits and resets the column headers.
14:30:08 <jralls> The only event that could do that is a charset change.
14:30:33 <jralls> tokenized != parsed into scratch transactions.
14:31:14 <jralls> And there's no point in creating the scratch transactions until the user clicks next on the CSV setup page.
14:32:17 <gjanssens> No you could also change the separator which would also invalidate the tokens, or switch from csv to fixed width data format (which I regret trying to squeeze into a single importer implementation)
14:33:09 <gjanssens> And the transactions are parsed because the preview gives live information about missing or invalid tokens. That's taken from the pretrans and presplit data
14:33:29 <gjanssens> BRB
14:36:59 <jralls> OK, but there's no need to re-tokenize to re-parse. In order to give live information I'd think it would need to reparse every time a column header changed type, but that seems not to happen.
14:40:39 *** sbluhm has joined #gnucash
14:50:12 <gjanssens> Back
14:55:14 <gjanssens> I *thought* there was also a path to only reparse when a column header changes, though IIRC it will only parse the changed column(s) not the whole
14:57:10 <jralls> There probably is. That would be good, and puts us back to not needing the scratch transactions while setting up the CSV.
15:02:08 <gjanssens> As I said, I think we do need to interpret the tokens to be able to give fast feedback to the user.
15:02:21 <gjanssens> Assume the user marks a column as date
15:03:06 <gjanssens> The pre-transaction parser will then go through all tokens in that column to check if these are valid for the chosen date format.
15:03:44 <gjanssens> This is a very simple case and could have been done by just a separate function.
15:04:45 <gjanssens> But there are also checks that require knowledge of multiple columns at once. Let me look for an example.
15:11:38 <gjanssens> Hmm, I can't immediately find one.
15:11:54 <gjanssens> So that's probably not why I did it.
15:13:28 <jralls> I had a breakpoint on GncTxImport::tokenize and didn't hit it when changing any of the column headers in Ove's test file.
15:13:39 <gjanssens> Where it does matter is in multisplit format where the parser has to decide where new transactions start. It has to interpret the values on multiple lines to determine whether they belong to the same transaction.
15:13:54 <gjanssens> That's what I would hope.
15:13:58 <jralls> Ove's test file is multisplit.
15:14:35 <gjanssens> I guess 6 of the triggers you had were from creating the field separator buttons (not verified, just a hunch)
15:15:19 <jralls> That would be bad, creating the buttons shouldn't have any side effects.
15:16:09 <jralls> If it's trying to guess which buttons should be selected it should do that once after setting up the UI.
15:17:00 <gjanssens> Yeah, still sifting through the code whether that could happen.
15:17:29 <gjanssens> Did you look at the bt for each of the times tokenize was hit ?
15:18:57 <jralls> But I can tell you what the functions were, I still have that note. In order: csv_imp_assist_prepare_cb, csv_txmip_preview_update_encoding, CsvImpTransAssist::preview_refresh, CsvTransAssist::preview_settings_load, csv_tximp_preview_settings_sel_changed_cb, and CsvImpTransAssist::preview_refresh (twice).
15:19:55 <gjanssens> Each of these triggered a call to gnxTxImport::tokenize ?
15:20:23 <jralls> Each calls it.
15:23:44 <jralls> That's wrong. Each is the 4th frame when the bp is hit.
15:23:44 *** Mechtilde has quit IRC
15:24:34 <gjanssens> Ok. Let's analyse.
15:24:59 <jralls> So e.g. preview_refresh()->cxv_tximp_preview_sep_button_cb()->CsvImpTransAssist::preview_update_separators()->GncTxImport::tokenize().
15:26:14 <gjanssens> update_encoding makes sense. Encoding changes require the input file to be reread which forces us to restart from scratch (though we try to reapply exising column assignments to the best of our knowledge)
15:26:37 <jralls> The catch is that I didn't change the encoding from the default.
15:27:16 <jralls> Nor did I change the separator. The only change was to tick the multiline box and to set the column headings.
15:27:39 <gjanssens> Yes, that was my other thing to go after. I think there's some sloppy initialization going on here.
15:28:23 <gjanssens> For example there seems to be a trigger of csv_tximp_preview_settings_sel_changed_cb
15:29:30 <gjanssens> I don't know yet where it comes from, but I suspect there's a call somewhere to explicitly start with the selection of "no preset selected" and that would trigger the callback.
15:30:41 <gjanssens> Which then would (unset) certain preview options like encoding or separator buttons triggering their callbacks as well.
15:31:29 <gjanssens> And those may have valid reasons to retokenize albeit during user interaction, not during initialization.
15:35:58 <jralls> That looks like a reasonable explanation for quite a few of them. Unfortunately GSignal doesn't have a "suspend" function so the only fix is to delay connecting the signals until after the UI is set up.
15:37:14 <gjanssens> Hmm, I thought signal handlers could be blocked ?
15:38:10 <gjanssens> https://docs.gtk.org/gobject/func.signal_handler_block.html
15:38:28 *** David has quit IRC
15:38:47 *** David has joined #gnucash
15:39:21 <gjanssens> https://docs.gtk.org/gobject/func.signal_handlers_block_matched.html can be used to find the proper calback functions.
15:39:51 <gjanssens> So indeed you can't suspend the signal, but we can make it ignore our callbacks as long as we don't want them to be fired.
15:40:30 <jralls> There's also https://developer-old.gnome.org/gobject/stable/gobject-Signals.html#g-signal-handlers-block-by-func.
15:41:34 <gjanssens> True, there are a few variations. Perhaps g_signal_handlers_block_by_func is more practical here.
15:41:50 <jralls> Right, I'd forgotten about the block functions. But it's less work to just delay connecting.
15:42:23 *** field^Zzz3 has joined #gnucash
15:43:23 <jralls> Although with a little creativity https://developer-old.gnome.org/gobject/stable/gobject-Signals.html#g-signal-handlers-block-matched could do all of them in one call.
15:54:45 <gjanssens> If you see a way, please do. I'm unfortunately not in a position to chase this. I can just add I think the place to do this is in the settings_sel_changed callback.
15:54:54 *** sbluhm has quit IRC
15:55:18 <gjanssens> It can block all preview refreshes until all settings are loaded and set and then run the preview update once.
15:55:44 <gjanssens> (preview_refresh)
15:55:51 <jralls> OK, I'll work on it in a bit, I'm looking into https://gitlab.gnome.org/GNOME/gtk/-/issues/3714 ATM.
15:56:59 <jralls> What do you think of the original question, though? That is, splitting up GncTxImport::tokenize into ::tokenize, ::parse_txns, and ::reset_column_headers?
15:57:05 <gjanssens> Heh, I have not worked that deep in the stack so hats off.
15:57:30 <gjanssens> (That was a reference to your work on gtk)
15:57:40 <jralls> Heh, I've been working on Gtk internals longer than on GnuCash.
15:58:23 <gjanssens> I think the split would only mean you'd have to call 3 functions everywhere we currently call only one (GncTxImport::tokenize)
15:58:45 <gjanssens> It may be the name is confusing and perhaps we should change that.
15:59:26 <gjanssens> And if you find the function is doing too much in one go, yes, we can split it in three smaller ones and wrap it with one grouping function.
15:59:27 *** field^Zzz3 has quit IRC
15:59:41 <jralls> Sometimes, but not always. For example changing the multisplit option only requires resetting the headers and reparsing, it doesn't change the tokens.
16:02:28 <gjanssens> As far as I can see changing multisplit doesn't trigger a tokenize ?
16:03:21 <gjanssens> Oh, it does, but that's because the assistant calls preview_refresh
16:03:52 <gjanssens> The importer state machine doesn't require to call tokenize again.
16:04:40 <gjanssens> So perhaps we should factor out the call to GncTxImport::tokenize from the preview_refresh function instead
16:04:41 <jralls> But I think tokenize is the only way to reset the column headers and re-parse the pre-txns.
16:04:52 <gjanssens> No it's not.
16:05:08 <jralls> And the other one is...?
16:05:36 * gjanssens may be responding too fast.
16:05:58 <gjanssens> Let me recheck
16:07:47 *** tomk_dk has quit IRC
16:11:32 <gjanssens> Take for example toggling the multisplit checkbox.
16:11:49 <gjanssens> This will trigger GncTxImport::multi_split(bool multi_split)
16:12:23 <gjanssens> In https://github.com/Gnucash/gnucash/blob/fbf828476f4c84a9978390e9192ee52878c99eab/gnucash/import-export/csv-imp/gnc-import-tx.cpp#L156 that function will check all set headers and adjusts them as needed without re-tokenizing
16:13:10 <gjanssens> Also the assistant's preview_refresh function doesn't explicitly call GncTxImport::tokenize as far as I can see.
16:14:01 <gjanssens> It does however toggle a lot of buttons without blocking signal handlers. So perhaps it is inadvertently triggering callbacks that do eventually call tokenize.
16:14:35 <gjanssens> I think preview_refresh should be adjusted such that it only makes gui changes without triggering callbacks.
16:14:41 <jralls> Doesn't it need to force a reparse?
16:14:58 <gjanssens> set_column_type takes care of reparsing
16:17:37 <gjanssens> That is the override with three arguments will, because it forces a (transaction related) column change
16:18:51 <jralls> Only to have its work thrown away when preview_refresh() calls tokenize. :-(
16:19:44 <gjanssens> Though for readability I think the part of that function that does the reparsing could be factored out into a separate function.
16:20:32 <gjanssens> Yeah, as I said before I don't think preview_refresh is supposed to call tokenize. That's careless gui changing.
16:21:20 <gjanssens> Note the separator checkbox gui updates are wrapped in callback blocking functions. But not all others are.
16:21:49 <jralls> The last line of preview_refresh directly calls csv_tximp_preview_sep_button_cb. No errant signal required.
16:22:37 <jralls> assistant-csv-trans-import.cpp:1705.
16:25:50 <gjanssens> Oh *there* it is. I really wonder why I did that.
16:26:34 <jralls> Doesn't that invalidate everything else done in the function?
16:26:38 <gjanssens> In any case I no longer think that's the proper place to call it.
16:26:49 <gjanssens> I think it does.
16:30:48 *** joo has quit IRC
16:33:07 <gjanssens> It looks like I changed the semantics of that function when I introduced loading and saving settings.
16:33:08 <jralls> Oh, preview_refresh does a whole bunch of setup that I'd think should go in the dialog's init function, including setting up signals.
16:34:13 *** joo has joined #gnucash
16:34:13 *** ChanServ sets mode: +v joo
16:34:40 <gjanssens> Which ones do you mean ?
16:34:51 <gjanssens> I only see blocking and unblocking of signals
16:35:10 <gjanssens> Unless you refer to the comboboxes that are generated as table headers ?
16:35:25 <jralls> No, I misread it. It blocks and unblocks, it doesn't connect.
16:35:44 *** kcin1 has joined #gnucash
16:37:08 *** kcin has quit IRC
16:37:59 <gjanssens> I think I was lazy by defering triggering the tokenize to preview_refresh (as a result of changing the separators) where it should be called in CsvImpTransAssist::preview_settings_load, right after it parses and sets the loaded settings.
16:38:12 *** kcin1 is now known as kcin
16:38:20 <gjanssens> preview_refresh should really not trigger a tokenize.
16:38:27 <jralls> Heh, in fact it looks like sep_button_cb isn't connected to a signal at all!
16:38:46 <gjanssens> It is, in the glade file.
16:38:58 <jralls> Ah, OK.
16:40:31 <gjanssens> So where we should end up: each callback for user actionable fields and buttons should performe updates to the GncTxImport state machine and then call preview_refresh
16:41:21 <gjanssens> The state machine updates can involve a re-tokenize if that's what the change needs. The preview_refresh should only reflect the state changes in the gui, not trigger any other state changes.
16:42:02 <gjanssens> So sep_button_cb should not be called from preview_refresh
16:42:23 <jralls> Does they need to call preview_refresh or just preview_refresh_table?
16:42:28 <gjanssens> Any code that depends on that function to trigger the tokenize function should instead call the tokenize function itself
16:42:46 <gjanssens> Good question.
16:43:09 <gjanssens> It probably depends on which state change that is triggered.
16:44:00 <gjanssens> For example toggling the multi-split checkbox requires updates to both the preview table and the base account field. So that one should call a full preview_refresh.
16:44:44 <gjanssens> Changing the number of lines to skip at start and end also require a full preview_refresh as either value change impacts the upper limit of the other value.
16:45:11 <gjanssens> I think trying to make a distinction here may be a premature optimalisation.
16:45:42 <gjanssens> Updating the button states is probably pretty fast.
16:47:52 <gjanssens> We could perhaps gain more by looking at settings changes that don't require a preview table reload, but I think there aren't any. Each settings change potentially changes the interpretation of the import data and hence the display of it.
16:48:14 <jralls> OK. I wonder about the file_format() conditional though. It enables the spearator controls if the format is CSV but doesn't disable them if it's not.
16:50:31 <gjanssens> Again a bit organically grown code. The function preview_update_file_format hides the checkboxes completely so it's irrelevant whether they are enabled or not.
16:50:58 <gjanssens> That showing and hiding is probably better moved to preview_refresh as well to keep gui updates in one place.
16:52:24 <gjanssens> In a more elaborate rewrite, I'd probably separate fixed with and csv format into two completely separate importer assistants.
16:52:57 <gjanssens> But that's not for anytime soon (tm)
16:54:00 <gjanssens> I wonder if we should block gui refreshes while resetting everything in preview_refresh for a smoother user experience.
16:54:14 *** kcin1 has joined #gnucash
16:55:04 <jralls> Doesn't that affect only the register?
16:55:27 <gjanssens> BTW do you know if gtk optimizes screen redraws ? I mean if a box is already visible and we invoke gtk_widget_set_visible(widget, TRUE) on it again, will GTK then skip redrawing ?
16:56:16 *** kcin has quit IRC
16:56:17 <gjanssens> Re blocking gui refresh, is that not something gtk provides in general ? I don't know those details too well.
16:56:41 *** kcin1 is now known as kcin
16:58:27 <gjanssens> Oh, does gtk's single-threadedness mean it will only perform a redraw when returning to the main loop after our preview_refresh has finished.
16:58:47 <gjanssens> Then there's not much blocking to do.
16:58:53 <jralls> gtk_widget_set_visible(true) just calls gtk_widget_show(); that checks the widget's visible flag and if it's already set it does nothing.
16:59:11 *** David has quit IRC
16:59:28 *** David has joined #gnucash
17:00:03 <gjanssens> Right, so it will avoid redundant redraws, which means I wouldn't worry too much by going over all our widgets in preview_refresh and set their states to reflect what's in our GncTxImport state machine.
17:00:41 <jralls> As for the single GUI thread, mostly, as long as you don't iterate the main loop to do something like update a progress bar.
17:01:17 <gjanssens> Understood. I don't think we do that currently in the csv importer preview.
17:04:07 <jralls> I haven't noticed a progress bar while testing.
17:04:21 <gjanssens> Note that preview_update_file_format also connects or disconnects a callback to the the treeview click event.
17:04:46 <gjanssens> This callback is needed in fixed width format as users need to be able to define where columns start and end.
17:04:55 <gjanssens> In csv mode this callback should not be there.
17:05:04 <jralls> OK.
17:05:15 <jralls> BIAB, I need to go work on my dinner rolls!
17:05:30 <gjanssens> I'm not sure whether connecting or disconnecting that should also happen in preview_refresh. I don't think so tobh.
17:05:55 <gjanssens> Enjoy dinner! I may be gone to bed when you're back.
17:06:13 <gjanssens> s/tobh/tbh/
17:07:15 *** kcin has quit IRC
17:09:55 *** joo has quit IRC
17:11:47 *** gjanssens has quit IRC
17:13:00 *** kcin has joined #gnucash
17:15:54 *** kcin has quit IRC
18:45:41 *** fell has quit IRC
18:46:00 *** fell has joined #gnucash
18:46:01 *** ChanServ sets mode: +o fell
20:03:47 *** User has quit IRC
20:31:42 *** celeste has joined #gnucash
20:31:42 *** ChanServ sets mode: +v celeste
20:33:42 *** fell has quit IRC
20:38:01 *** fell has joined #gnucash
20:38:02 *** ChanServ sets mode: +o fell
21:03:19 *** alt_mikee has quit IRC
21:05:42 *** mikee has joined #gnucash
21:42:02 *** KipITOne has quit IRC
21:45:07 *** KipITOne has joined #gnucash
21:45:08 *** ChanServ sets mode: +v KipITOne
22:19:29 *** thechitowncubs has joined #gnucash
22:19:29 *** ChanServ sets mode: +v thechitowncubs