Tuesday, 2022-03-29

dcalisteGood morning pvuorela.07:07
pvuorelagood morning07:07
dcalisteI pushed yesterday a new version for notebook change in mKCal where deleted incidences are purged before performing the delete + insert pattern. But thinking about it during the night, while working ok when changing back, bring me to the conclusion that it will not work in cases neither :/07:10
dcalisteWe need to be able to leave deleted incidences for every notebook where it had once belonged to.07:10
dcalisteThen, it's up to the sync plugin to purge them when the deletion has been propagated to the server.07:11
pvuorelasounds about right.07:11
dcalisteThis leads me to think that the purgeDeletedIncidences() function should take a notebook parameter.07:11
dcalisteBecause on insertion, we should restrict the purge to incidence of the given notebook only.07:12
dcalisteBut this would be an API break…07:12
dcalisteAt the moment, the sync plugins are purging all deleted incidences matching UID+RecID, which would break any deleted + inserted pattern.07:13
dcalisteSync plugin should delete incidences following UID+RecID+NotebookId…07:13
pvuorelauh, right.07:14
pvuorelathis might get hairy if it's possible to have duplicated instances in different notebooks.07:15
pvuorelawonder would we have some path of separating the notebooks better.07:16
dcalisteYes, but that's the only "simple" way to get deletion notification in sync plugins… For a given UID+RecID, it can exist as deleted for any notebook and not deleted for only one.07:17
dcalisteThe current UNIQUE constraint is compatible with this assomption as long as the deleted ones are separated by at least one second ;)07:18
dcalisteWhat do you think of the following plan:07:20
dcaliste- implement the notebook argument in purgeDeletedIncidences() (API break),07:20
pvuorelasorry phone call...07:21
dcaliste- test the viability of it to get several deleted incidences of the same UID+RecID for various notebook and only one for non deleted,07:21
dcaliste(no problem, come back when the call is finished, or simply when you have time to)07:22
dcaliste- propagate the API break to sync plugin and QML bindings.07:22
pvuorelaback. mixed feelings, might be possible to get the case working and the purgeDeletedIncidences could be special enough to adjust everywhere where it's used now (i.e. sync plugins and qml). but then again this still wouldn't allow to have duplicated non-deleted events in different notebooks.07:32
pvuorelaon feasibility, the qml side worker seems to check notebook with calendar::notebook(incidence pointer) on storageUpdated(), wonder how well that would work if the event is deleted in many.07:33
dcalisteYes, notebookID can be retrieved from calendar, as you mention, but that's only for incidences that are loaded in this calendar (deleted or not). And currently, mKCal is not loading deleted incidences in a calendar from DB, and sync plugin are using purgeDeletedIncidences() with incidences coming from storage::deletedIncidences() which don't belong to the calendar, by definition.07:36
pvuorelabtw, having another meeting coming up in 25 minutes.07:36
dcalisteSo there is no incidence <-> notebook association possibility when we are in purgeDeletedIncidences().07:36
dcalisteThat's why, I'm suggesting to add a notebook argument.07:36
dcalisteI don't like it much neither, because it's not self contained (you can pass valid incidences that does not belong to this notebook for instance).07:37
dcalisteBut I don't see any other possibility at the moment, snce KCalendarCore::Incidence has no notion of notebook.07:38
dcalisteAnd yes, this change would not allow to have several notebooks for a given UID+RecID pair.07:39
dcalisteBut actually, this would break also the calendar upstream API. It's a 1:1 association between notebook and incidence.07:39
dcalisteIf we want to have this and compatible with upstream API, we need to move to one calendar per notebook. Which is a huge change…07:40
pvuorelahaven't planned it, but pondering how it would go if we reconstruct the whole thing. something like separate calendar+storage for each notebook. some class for enumerating the different notebooks on higher level.07:40
dcalisteWhy not. But it will be a huge endeavour. That would be an interesting move though. Can think about it. In the meantime, I'm more and more thinking about decoupling completely the sqlitestorage from calendar. As we discussed last time, just emit signals and let the user code populate a calendar or pass it to another thread.07:43
dcalisteI think this can be done while keeping the current extendedstorage API ok for transition, but by unplugging the sqlitestorage under hood and use the signals to populate a calendar at the extendedstorage level.07:44
pvuorelasure. let's see how that works with code using it. as commented on the current mkcal PR, it seemed small enough not to require immediate merge alone.07:46
pvuorelaand on the kf5-calendarcore on the tagging date it looked like a new version might be coming in near future. has been beginning of the month earlier.07:47
dcalisteI agree. I can stay as an opened PR at the moment as a play ground for experiments.07:47
dcalisteFor KF5, as you say, it can wait for the official version 5.93. I'll update the PR at that moment if you prefer so.07:48
dcalisteI've noticed an issue yestarday evening with latest QML binding. I've introduced a regression I think where all events are displaying organizer now in the UI. I'm going to fix this in the coming days if it's actually a regression.07:50
pvuorelaalright, thanks in advance already :)07:51
dcalisteNo problem, I need to go and buy a good UBS C cable first, the one I bought is a cheap one that is transmitting data only once in a while :/07:53
pvuorelasure. still new enough connector so not yet having piles of random cables :)07:55
dcalisteExactly, and 3€ was maybe not a good gambling.07:56
dcalisteThank you for the discussion. I'm going to continue to play with mKCal reorganising and see what comes out of it. And see to fix the organizer issue in UI.07:57
dcalistepvuorela, when you are free, try to add this line "Component.onCompleted: console.warn("init " + keyboard.language + " -> " + language)" in Xt9InputHandler.qml in Xt9EngineThread{} and restart maliit-server. You'll notice that the language is not set properly (at least on my device).08:48
dcalisteFor me it's outputting : "WARNING: init FR -> "08:48
dcalisteSomething inside Xt9EngineThread seems to reset to empty the property language after the QML has assigned it to keyboard.language, and before the component is completed.08:49
dcalisteTherefore the French typography rules are not properly applied… see https://forum.sailfishos.org/t/missing-space-before-punctuation-in-french-typing/1082408:50
pvuoreladcaliste: hm, indeed there's something. i'll check in more detail.09:02
dcalisteThank you. The simplest way I've found to solve the user facing issue was to replace `thread.language` by `keyboard.language` on line 324, but maybe it's hidding some deeper bug.09:04
pvuoreladcaliste: yea, there seems to be a deeper bug but simple thing. testing a fix now.09:13
dcalisteOh, super great ! Thanks a lot.09:14
Thaodanhttps://openrepos.net/content/ichthyosaurus/patch-xt9-improved-punctuation-handling helps with this09:41
ThaodanAlso in the case of German09:41
dcalisteIndeed, thank you Thaodan, I got the inspiration from him mentioning his patch on the Forum. But the current problem was that the language property was not set properly and thus the tweaking on language was not working.09:48
dcalisteWhat is strange also, but not mentioned yet is that without the patch, ':;' characters are working while the code there contains only a test on '?!' characters. I guess Xt9 was already properly handling ';:' characters ?09:49
*** Mister_Magister_ is now known as Mister_Magister11:12
attahChoo-choo, all aboard the hype-train :)19:45

Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!