Bug799179 SLR Troubles #1846
Bug799179 SLR Troubles #1846
Conversation
|
This worries me because the SLR isn't modal so the user can do things that cause the instances list to change resulting in a dangling pointer in the tree model. But looking through the code to check that showed me so many other resource-management issues in the SX model that I don't think this one case makes a crash significantly more likely than it is now. |
|
Hmm, I assume you mean the SLR could be affected by changes to the schedules in the editor if the SLR was still open. Well this got me thinking and tried doing some changes. Updates and additions of schedules result in the SLR model being repopulated but I had missed a code path for deletions in |
|
What I'm particularly concerned about is deleting or disabling an SX in the SX editor while the SLR is open: That will free the |
This was due to a previous commit to remove empty instances. It was not realised that the GtkTreeModel needed to be in sync with the GList of instances as the location in the model was being used to find the location in the GList. To fix this, a new column 'PTR' was added to the model and populated with instances, instance or variable depending on depth and these are used to the required objects.
As a tree model is being used, the transaction column displays either the schedule name at depth 1 or the occurrence date at depth 2. This allows for the sorting function to be changed on path depth so that the sorting can be done by schedule name or occurrence date. To sort by schedule name, a schedule name is first selected and then the column header is pressed to change order. To sort by occurrence date, a date is selected and then the column header is pressed to change order based on the date of the first occurrence. A tool tip has been added to indicate the sort order being used.
|
I have fixed the missing code path mentioned above and made changes to the sorting. I have removed the status sorting as I think that was too confusing and could not see a clear way without adding complexity. I have also changed the sorting on the transaction column. As this is a tree model, that column displays either the schedule name or occurrence dates so have used the path depth to change the sorting. If a schedule name is selected and sorting adjusted then the sorting will be based on the schedule name. If a date is selected, then the dates are sorted and the schedules are sorted based on the first occurrence. I have also added a tool tip to the column that indicates this but wording may need improving.
|
|
From what I can tell all the model updates are from signals and not queued from an idle. Adding and updating schedules result in the model being repopulated and deletions delete the appropriate model row. The instances, instance and variable pointers are used in user tree view updates. Instances are also used to find the row to delete. These actions stem from the event handler in I have tried various actions with the SLR open, deleting single and multiple schedules, updating the schedule by changing the formula, disabled and enabled a schedule and deleting an account with schedule all appeared to work. |
|
Is there anything missing here or is there something a newb like me can help to get this merged and released (e.g. testing)? currently I can't even enter prices values for scheduled transactions and this seems like it would fix it |
I think this will fix the SLR troubles...
In the original code the GtkTreeModel is populated from a GList of GncSxInstances. The path of the GtkTreeModel entry is split in to indices and various parts are used to do a lookup in the GList. As I was not adding the empty instances, the two lists will be out of sink and depending on where the missing instances were affected what fault was highlighted in the SLR.
To fix this, I added another column to the model that holds a pointer to instances, instance and variable depending on the depth and these values are used to look up the required objects.
The second commit removes an unused function, I did notice there are some 'g_warning' entries so I suppose they should be changed to 'PWARN' entries.
To test this I changed the order of schedules in my XML test file so it resulted in this for 5.4 ...
And after this change ...
I have been able to change any of the entries with correct results but I thought that last time !!!
What I did notice but this existed before these changes is...
If you have entries after one that is in a state 'Postponed' and try to update them, they are not removed from the SLR as shown below when I changed 'Test-24/01/2024' to create and provided a value for 'Avar-28/12/2023', pressed OK and rerun SLR which resulted in the following...
So I am wondering if this is a 'BUG' and should the following be applied in another PR/commit...
If entry changed to 'Postponed', all entries after should be changed to 'Postponed'
If entry changed from 'Postponed', all entries before should also be changed to that state.
This may also apply for 'Reminder' changes.