Tuesday, 2021-12-07

riniguspiggz: looking good! I'll comment in the issue06:35
dcalisteGood evening chriadam, how are you ?08:03
chriadamhi dcaliste, I'm well thanks - how are you?08:03
dcalisteI'm good thanks. With 4.4, the UI to display sync logs is working out of the box, thanks for previous merges. To complement it, I've two small PRs:08:05
dcaliste- https://github.com/sailfishos/buteo-syncfw/pull/8 to make invocable the function that get the log message,08:06
dcaliste- and https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/15 to get back the instance identifier from the model (identifiers are input, but were not available as data for each entry).08:07
dcalisteWith these two commits, it's then possible to fetch the error message for a failed item from the logs.08:07
chriadamwe should have a bug number which can be used for these commits08:08
chriadamlet me try to find it08:08
dcalisteI've polished https://github.com/dcaliste/harbour-logbook and made a release. If you want to try, just install it and you should be able to browse your logs.08:08
chriadamthanks, I will take a look this week08:08
dcalisteSure, I'll add the bug id tag. Thank you.08:09
chriadamthose two seem quite simple.  one the bug tag is added, I will merge and tag (probably tomorrow)08:10
chriadamthank you08:10
dcalisteAbout calendars, you've already approved https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/13 if I remember correctly, right ?08:11
chriadamyes, but I was hoping that pvuorela would also double check it08:12
chriadamsince he had a variety of comments on the OMP PO08:12
chriadamI will ask him to check, sec08:12
dcalisteSure. Thank you.08:12
dcalisteAbout those two PRs for sync logs. Actually I've already added the bug id to the commit messages.08:14
chriadamgreat - thank you08:15
dcalisteIt's not in the PR headers because I guess I thought about it after submitting the PR.08:15
dcalisteAnother "simple" one, but that require some discussion is this one : https://github.com/sailfishos/buteo-sync-plugin-caldav/pull/908:16
dcalisteI made a mistake when putting the cancel events on update to the to be deleted list. I put it in the list of locally deleted ones that needs to be synced to the server.08:16
dcalisteLuckily, the sendLocalChanges is called before and this is never commited.08:17
dcalisteBut it appeared in the log as a local deletion, which made me investigate.08:17
dcalisteBut that's not the point of the discussion : before my mistake, the incidence was deleted by a direct call. But I think it's not completely correct to delete locally cancelled events.08:18
dcalisteActually, mKCal is already not applying alarms for deleted events.08:18
dcalisteSo it may be better to keep the event on device and have a way in UI to mark it as cancelled. Something with an enabled: false somewhere.08:19
dcalisteWhat do you think ?08:19
chriadamis this not already possible?08:19
chriadamwe have something like this for exchange events, I think, at least?08:19
dcalisteIn the RFC, cancelled events are adviced for organisers to push this to invitees.08:19
dcalisteI don't know, I'm not using Exchange :/08:20
dcalisteSo maybe, it would be better to keep the update for cancelled events instead of deleting them in CalDAV sync ?08:21
chriadamI wonder if flypig knows.  I suspect there might be a participation status which specifies that the event was cancelled, perhaps08:21
chriadamI think you're right: deleting them altogether is probably unwanted08:21
chriadamthe user can manually delete the cancelled event, if they choose.08:21
flypigSorry, I've not been following, will try to catch up.08:21
dcalisteOk, so I'll amend the commit to completely remove the deletion in case of cancelled events.08:22
dcalisteHello flypig, it's about cancelled events. (STATUS: CANCELLED in iCal format).08:22
dcalisteWhat to do with them when upstream is applying this status.08:22
dcalisteAt the moment, CalDAV is deleting them. And we're wondering how Exchange and Google are handling this.08:23
chriadammostly I was wondering what we do in exchange plugin case.  would be good to try to have same semantics.08:23
dcalisteAnd also (!), if there is a UI feedback already for cancelled events.08:23
flypigOkay, that all makes sense at least. I'm not sure what the correct behaviour is I'm afraid. At least in Thunderbird, cancelled EAS events aren't deleted.08:24
piggzflypig: oh, forgot about Q's ... will do today!08:24
dcalisteYeh, I think that's a mistake to delete them in CalDAV sync. Alarm won't ring already for cancelled events. I need to try to see if there is an additional visual feedback or not… But at least I'll change the PR not to delete them.08:25
chriadamthank you08:26
chriadamI will ask pvuorela and mschuele if some specific UI indication exists (I think not - in exchange case the subject usually has "CANCELLED" prepended) and if not, what might be done (new icon?)08:26
flypigFor EAS events, Thunderbird draws a line through the title. Jolla-calendar prefixes with "Cancelled", but there doesn't seem to be any other indicatin.08:27
flypigAnd I suspect the "Cancelled" prefix comes from the server changing the title, so probably we should be adding a local indication too.08:27
chriadamflypig: do we set partstat to cancelled or something?08:28
dcalisteWell, having it marked as "caneclled" in the summary is already good enough in my opinion. A bit of Theme.opacityDisabled or whatever is called on the notebook color would make even more clear maybe.08:28
chriadami.e. is there a field which we can already use to show the indicator?08:29
dcalisteAh, yeh, if the cancelled string is set by the server, that may be too peculiar to server which actually does it…08:29
chriadamsp/use to show/use in order to determine if we need to show, I meant08:29
flypigdcaliste, I think the problem with relying on the title, is that this is like a "Fwd" in emails: it can probably be edited away. E.g. I have ones with "Peruutettu" in Finnish rather than "Cancelled".08:30
dcaliste; -) indeed. So playing with the theme opacity and having a dedicated Label in EventViewPage.qml would be more appropriate maybe.08:31
dcalisteLooking at CalendarEvent class in nemo-qml-plugin-calendar, I'm surprised that the status is not exposed…08:31
dcalisteSo, currently, chriadam, I would say that there is no way in QML to know that an event has been marked as cancelled.08:32
chriadamme too08:32
flypigIt sounds like it wouldn't be hard to expose it. The info is retained internally.08:33
dcalisteSo maybe, keep deleting them in CalDAV makes sense, otherwise the user may get confused by an event that is still on device (with alarm visible in EventViewPage) but that don't ring.08:33
dcalisteflypig: indeed, as for the rest, it needs to be duplicated in CalendarData::Event structure and then exposed in the Event object.08:34
chriadamdcaliste: I would prefer not deleting them, personally, but ensuring that the status is set appropriately.  I would even not be against prepending "Cancelled" (localised) to subject if the event subject doesn't contain it.  -- just until we can do the indicator more properly.08:34
flypigIt would also make sense to be consistent across EAS and CalDav.08:35
dcalisteRight. So I'll change the sync plugin in CalDAV to keep them and update them with the flag. I'll prepare a PR in QML plugin to expose the status properly also.08:36
chriadamthat would be greatly appreciated08:36
flypigYeah, that sounds really nice.08:37
chriadamnot sure when MartinS will get a chance to look into the design side, though08:37
dcalistePrepending "cancelled" in subject in the sync plugin is a bit ugly though… Particularly if it requires localisation. :/08:37
chriadamindeed, fair enough, let's avoid that for now08:37
dcalisteAnother subject: I've a bunch of PRs now to add UI feedback also for failing events. I've begun to see how one could help the user to recover from not fatal errors on specific events.08:38
dcaliste- https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/16 I'm adding yet another flag so the user can give indications to the sync plugin for failing events,08:39
dcaliste- https://github.com/sailfishos/buteo-sync-plugin-caldav/pull/10 the sync plugin can discard flagged events to avoid retry always-failing events.08:40
dcaliste- and jolla-calendar#312 to add a combo in the EventViewPage for failing events to help the user indicate what to do.08:40
dcalisteThe sync status (for CalDAV) now is that single error on events are not fatal and the sync is marked as a success. Failing events are flagged as failing and marked in the UI and in the logs.08:41
dcalisteThe three above PRs add an option to the user to indicate to the sync plugin if the failing event should be retry at next sync or ignore and kept out of sync.08:42
chriadamthe nqpc and caldav PRs look simple enough and I agree with.  the jolla-calendar one I agree with in principle, but I guess the exact UI change needs discussion from the UX experts.08:43
dcalisteI would like to complement the UI PR also with a possibility in the combo to choose to force the deletion on device (for failing upload) or to reset to server version (for failing upload also).08:43
chriadamthank you very much for creating the PRs08:43
chriadamyes, those extra possibilities would be useful.  requiring more work, though.  This is a very good first step.08:44
dcalisteIndeed, I was hoping for some feedback beforz adding more options.08:44
dcalisteTechnically they are not that complicated, it's just about setting more values to the flags and react accordingly in the delta calculation.08:45
dcalisteI'm wondering in particular about the possibility to add modification to an event (here with the combo for flagging them) in the EventViewPage.08:46
dcalisteThat's what seems to me to be the best place (the edit page being already much crowded) and going well with tyhe warning label : the user see the label and is proposed a choice to get rid of it.08:47
dcalisteBut I'm open to discussion, of course.08:47
chriadamit does seem like a natural plae for it.08:47
chriadamI agree.  but, yeah, let's see what Joona and Martin suggest.08:48
chriadamyou've been very busy these last few weeks - thank you very much, as always, for your efforts.08:49
dcalisteYes, indeed, thank you.08:49
chriadamare there any other PRs which I need to put on my list to review?08:49
dcalisteAnd coming (almost) last, the infamous : https://github.com/sailfishos/sailfish-secrets/pull/18208:50
chriadamoh, let me check08:50
dcalisteI'm a bit ashamed to see that I messed this up completely.08:50
dcalisteWhat I thought was a fix for some error I got with signature was a complete disaster : it killed the security-ui completely.08:51
chriadamhaha no problem08:51
chriadamI will push to 4.4.x08:51
dcalisteI've no idea how I got the error first, and how I thought it was solved then with this.08:51
chriadamI thought I tested the original PR and it worked fine08:51
chriadamall good08:51
dcalisteRestarting from scratch on and recompiling secrets and ensuring that it's the right version running, this time it's working (with the revert).08:52
dcalisteSo sorry for the loop.08:52
chriadamno problem at all08:53
dcalisteThank you.08:53
chriadamgive me a sec, triggering it now08:53
dcalisteAfter this one, there are just two minor doc updates :08:54
dcaliste- https://github.com/sailfishos/docs.sailfishos.org/pull/1808:54
dcaliste- and https://github.com/sailfishos/docs.sailfishos.org/pull/1908:54
dcalisteI noticed these while browsing the docs looking for something else.08:54
dcalisteIt's quite complete actually. Thanks to sledges and those who made this possible.08:55
chriadamyes, it's really nice that it's on github now!08:57
chriadamthanks for those.  I've approved one, and added a comment to the other08:57
chriadamthank you for those improvements08:57
dcalisteI've seen, I'll update it according to your suggestion.08:57
chriadamUnfortunately, I have to run to another meeting now - but I guess we are finished for today?  Thank you again for your time and efforts - it is very much appreciated.08:58
chriadamI will ping Pekka about that nqpc one which OMP need.08:58
chriadamand poke Martin and Joona about the jolla-calendar UI thing.08:58
dcalisteYes, it was the end of the list. Thank you for taking time to review and discuss. We've moved forward for the cancel case stuff. That's good.08:58
chriadamthanks - I hope you have a great week!08:59
* chriadam -> away, gnight!08:59
dcalisteThank you, you too.08:59
Thaodanrinigus: Can you create python3-tomli-w for me in chum?09:09
piggzrinigus: yeah, feel free to look at the github api part09:52
rinigusThaodan: later tonight, sure09:53
riniguspiggz: great, will do09:53
Thaodanrinigus: Thank's that was the last one :D09:54
piggzrinigus: re BEGINCHUMMETADATA ..... I see yur point, however 1) it make it explicit that metadata exists, if you just hope that the last para contains metadata then there is a chance of issues.  2) it makes parsing super easy!09:55
piggzother option would be something like findLastIndexOf("\n") ?09:57
piggzor probably second last?09:58
piggzno, would be \n\n for new line09:58
piggzs/blank line09:58
Thaodanpiggz: which metadata?10:19
Thaodanah just read it. Did you consider using appstream as metadata?10:20
ThaodanOnly downside is that it's xml based10:21
ThaodanQt backend also exists10:22
piggzThaodan: yes, i asked rinigys about using appstream, and he wasnt in favour of it as it was a pita when used in flatpak iirc10:33
piggzthis atlleast is quite simple for dev to add to a package10:33
piggzplus, how would we get the appstream data using nothing but an OBS repo10:38
riniguspiggz: I think we should first parse description into list of paragraphs (\n\n, merge splits if needed like in \n\n\n) and then try to parse the last one assuming it is yaml. if it fails, too bad - no metadata. that way it will all look nice in regular zypper info as well11:00
rinigusThaodan: as you asked in github as well, I'll better reply there.11:00
riniguspiggz: one more note regarding parsing description: the paragraphs above yaml can be probably merged into single line of text (one line per paragraph). that would make line brakes on phone to follow display size - something we probably expect from description11:10
Thaodanpiggz: From what I read they can be generated https://blogs.gnome.org/hughsie/2016/04/27/3rd-party-fedora-repositories-and-appstream/11:10
KetoOBS seems to have some handling for the appdata.xml, but don't know if and how it works11:53
Ketobut imho, that would seem like a saner option, if it works, than hacking something in the rpm descrioption11:54
ThaodanWell if hacking that into rpm would so easy it would have been done imho.13:24
ThaodanKeto: Do you know the require OBS version?13:28
Ketogoogle gave some blogpost about obs 2.4 release which mentions appstream support, so it should be there13:36
Ketobut I did not dig deeper13:37
piggzals lbt :D13:37
Ketoafaik the community obs is 2.913:37
rinigusThaodan: re "if hacking ... easy" comment - do you foresee some issues with the proposed approach?14:07
Thaodanrinigus: Duplication of metadata and character limit/encoding issue. The description field isn't made for what you want to do.14:08
Thaodanobs devs say running appstream over the rpm-md repo will create/extract any appstream metadata.14:09
rinigusThaodan: problem is that SPEC does not provide all metadata which is needed. The missing data are links to  icon, screenshots, issues and discussion forums (from the issue at github). in addition, app name (name used in .desktop) is even not there. so, generation of appstream from rpm will not cut it.14:13
ThaodanYes and all this is  done by appstream. Why do you want to duplicate that work? Note you linked your puremaps appdata file which is far more verbose then the standard appdata.xml.14:15
Thaodanappstream builder builds the appstream db from the metadata in the packages that ship appdata.xml. That data can be simply downloaded as if you would do it from openrepos for its metadata or the rpm repo db.14:16
rinigusThaodan: re pure maps appdata - that was required by flathub for pure maps distributed there.14:18
rinigusThaodan: at this moment, we have pkcon provided info regarding chum packages. to "simply download", I need the location from which I can fetch it then. so, every dev will have to put it out somewhere in their repos. that would work, in principle, just one hop more in the app.14:21
rinigusThaodan: regarding limitations - what is the char limit for description field. note that we try to avoid duplication of metadata by adding only missing bits into SPEC description field14:22
rinigusThaodan: char encoding - don't know much about encoding supported by SPEC. Is UTF8 supported?14:22
ThaodanOK So they only issue left is that you don't like xml? At least for appstream there's no location but only the rpm-md that is read, packages anylized for existing appstream data and then the db created. For your parsing of the description field you would do the same.14:27
Thaodanrinigus: yes utf-8 works14:27
rinigusThaodan: I was asking about char limits in SPEC description field and whether UTF8 is supported there. those were the limitations that you listed above. as for xml - we can take that discussion separately14:30
rinigusas such, I am still preferring adding few fields to SPEC instead of full appdata. note that full appdata is a duplication of the same metadata as in spec, in part of it. for example, description field14:32
rinigusbut if you know SPEC char limits and encoding limitations, please tell14:32
Thaodanrinigus: I don't know exactly just pointed out if it was so easy to extend rpm metadata this way it would have been done. the rpm spec file it self at least support utf8.14:41
ThaodanIf you want to know more ask #rpm.org on libera or github14:41
rinigusThaodan: just looked at suse spec guidelines and found that utf8 should be fine. for description, no char limit is given, as far as I can see, only recommendations. in this sense, there doesn't seem to be technical limitations as such.14:45
piggzwhen i looked it up, rpm5 adds arbitrary fields, but we are on rpm414:45
Thaodanpiggz: Yeah it did the same, a spec file is so nice to retrieve appdata from similar how to importlib.metadata14:46
XenoPLHi, quick question to piggz: is there any hope advanced cam is gonna be fixed with/after SFOS 4.4? (IIRC it supposed to have new camera API available that would allow apps fiddling with advanced camera properities).14:48
piggzwas hoping to discuss the issue with karry ... you here?14:49
Thaodanrinigus: Can you add python3-tomli-w to chum for me?16:58
rinigusThaodan: was just pasting it into github form at the moment I got your ping :)16:59
ThaodanxD thanks16:59
rinigusThaodan: done!17:00
*** Mikaela is now known as Guest788921:15
piggz_rinigus: pushed branch to main repo, now parses last paragraph22:23

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