| dcaliste | Good morning pvuorela. | 07:07 |
|---|---|---|
| pvuorela | good morning | 07:07 |
| dcaliste | I 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 |
| dcaliste | We need to be able to leave deleted incidences for every notebook where it had once belonged to. | 07:10 |
| dcaliste | Then, it's up to the sync plugin to purge them when the deletion has been propagated to the server. | 07:11 |
| pvuorela | sounds about right. | 07:11 |
| dcaliste | This leads me to think that the purgeDeletedIncidences() function should take a notebook parameter. | 07:11 |
| dcaliste | Because on insertion, we should restrict the purge to incidence of the given notebook only. | 07:12 |
| dcaliste | But this would be an API break… | 07:12 |
| dcaliste | At the moment, the sync plugins are purging all deleted incidences matching UID+RecID, which would break any deleted + inserted pattern. | 07:13 |
| dcaliste | Sync plugin should delete incidences following UID+RecID+NotebookId… | 07:13 |
| pvuorela | uh, right. | 07:14 |
| pvuorela | this might get hairy if it's possible to have duplicated instances in different notebooks. | 07:15 |
| pvuorela | wonder would we have some path of separating the notebooks better. | 07:16 |
| dcaliste | Yes, 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 |
| dcaliste | The current UNIQUE constraint is compatible with this assomption as long as the deleted ones are separated by at least one second ;) | 07:18 |
| dcaliste | What do you think of the following plan: | 07:20 |
| dcaliste | - implement the notebook argument in purgeDeletedIncidences() (API break), | 07:20 |
| pvuorela | sorry 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 |
| pvuorela | back. 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 |
| pvuorela | on 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 |
| dcaliste | Yes, 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 |
| pvuorela | btw, having another meeting coming up in 25 minutes. | 07:36 |
| dcaliste | So there is no incidence <-> notebook association possibility when we are in purgeDeletedIncidences(). | 07:36 |
| dcaliste | That's why, I'm suggesting to add a notebook argument. | 07:36 |
| dcaliste | I 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 |
| dcaliste | But I don't see any other possibility at the moment, snce KCalendarCore::Incidence has no notion of notebook. | 07:38 |
| dcaliste | And yes, this change would not allow to have several notebooks for a given UID+RecID pair. | 07:39 |
| dcaliste | But actually, this would break also the calendar upstream API. It's a 1:1 association between notebook and incidence. | 07:39 |
| dcaliste | If 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 |
| pvuorela | haven'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 |
| dcaliste | Why 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 |
| dcaliste | I 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 |
| pvuorela | sure. 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 |
| pvuorela | and 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 |
| dcaliste | I agree. I can stay as an opened PR at the moment as a play ground for experiments. | 07:47 |
| dcaliste | For 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 |
| dcaliste | I'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 |
| pvuorela | alright, thanks in advance already :) | 07:51 |
| dcaliste | No 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 |
| pvuorela | sure. still new enough connector so not yet having piles of random cables :) | 07:55 |
| dcaliste | Exactly, and 3€ was maybe not a good gambling. | 07:56 |
| dcaliste | Thank 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 |
| pvuorela | thanks | 07:58 |
| dcaliste | pvuorela, 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 |
| dcaliste | For me it's outputting : "WARNING: init FR -> " | 08:48 |
| dcaliste | Something 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 |
| dcaliste | Therefore the French typography rules are not properly applied… see https://forum.sailfishos.org/t/missing-space-before-punctuation-in-french-typing/10824 | 08:50 |
| pvuorela | dcaliste: hm, indeed there's something. i'll check in more detail. | 09:02 |
| dcaliste | Thank 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 |
| pvuorela | dcaliste: yea, there seems to be a deeper bug but simple thing. testing a fix now. | 09:13 |
| dcaliste | Oh, super great ! Thanks a lot. | 09:14 |
| Thaodan | https://openrepos.net/content/ichthyosaurus/patch-xt9-improved-punctuation-handling helps with this | 09:41 |
| Thaodan | Also in the case of German | 09:41 |
| dcaliste | Indeed, 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 |
| dcaliste | What 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_Magister | 11:12 | |
| Nekron[m] | https://blog.jolla.com/sailfish-os-vanha-rauma-brings-in-several-new-features-and-improvements/ | 19:40 |
| attah | Choo-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/!