treesheets
treesheets copied to clipboard
aardappel
Crash when deleting a grid using a script
Full description in the crash.cts, use crash.lobster as the script to run. (zip contains crash.cts, crash.lobster) crash.zip
The solution would probably be to forbid the deletion of grids that are on the path toward the selection grid from the root cell. I have not tried if the crashes happen with deletions outside of the drawroot, even if they do not delete the grids entirely.
First, I don't thing crash.lobster was working correctly, because size/pos were inverted. I updated the ts_selection doc to reflect this.
Spent a while debugging this, and didn't come to a clean solution. The problem is that all TreeSheets code was written with user actions in mind, and the script can arbitrarily move the selection in ways that it doesn't expect, which causes these crashes.
For now, I changed ts_delete to clear the selection and zoom all the way out. I tried various ways of making it update the selection/drawroot consistently, but none always worked. So this is of course not a great solution, you're welcome to try and improve. At least for now it doesn't crash or land you in an inconsistent state.
https://github.com/aardappel/treesheets/commit/39d367e0458a4b2879cffe9a3317e84ff8ab45c0
Oh right, I completely forgot to mention that I un-inverted the the pos and size in my fork.
As for ts_delete, my idea was to let scripts move the selection via either moving the drawroot to the point where it is visible, or setting the selection to be the drawroot. Then, there would be a check if the deleted grid is on the drawpath, in which case the deletion will fail.
For the former selection moving behaviour I would need to understand fully how the rendering works, but when I looked into it I found that it didn't work quite as it is supposed to IMO. For more info, see https://github.com/aardappel/treesheets/issues/235#issuecomment-1153110231
Yes, it is complicated. Currently selection and drawroot get patched up in each individual action where needed based on user action. Would have been good if that was done more generically.