| rinigus | piggz: looking good! I'll comment in the issue | 06:35 |
|---|---|---|
| dcaliste | Good evening chriadam, how are you ? | 08:03 |
| chriadam | hi dcaliste, I'm well thanks - how are you? | 08:03 |
| dcaliste | I'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 |
| dcaliste | With these two commits, it's then possible to fetch the error message for a failed item from the logs. | 08:07 |
| chriadam | we should have a bug number which can be used for these commits | 08:08 |
| chriadam | let me try to find it | 08:08 |
| dcaliste | I'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 |
| chriadam | JB#49486 | 08:08 |
| chriadam | thanks, I will take a look this week | 08:08 |
| dcaliste | Sure, I'll add the bug id tag. Thank you. | 08:09 |
| chriadam | those two seem quite simple. one the bug tag is added, I will merge and tag (probably tomorrow) | 08:10 |
| chriadam | thank you | 08:10 |
| dcaliste | About calendars, you've already approved https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/13 if I remember correctly, right ? | 08:11 |
| chriadam | yes, but I was hoping that pvuorela would also double check it | 08:12 |
| chriadam | since he had a variety of comments on the OMP PO | 08:12 |
| chriadam | I will ask him to check, sec | 08:12 |
| dcaliste | Sure. Thank you. | 08:12 |
| dcaliste | About those two PRs for sync logs. Actually I've already added the bug id to the commit messages. | 08:14 |
| chriadam | great - thank you | 08:15 |
| dcaliste | It's not in the PR headers because I guess I thought about it after submitting the PR. | 08:15 |
| dcaliste | Another "simple" one, but that require some discussion is this one : https://github.com/sailfishos/buteo-sync-plugin-caldav/pull/9 | 08:16 |
| dcaliste | I 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 |
| dcaliste | Luckily, the sendLocalChanges is called before and this is never commited. | 08:17 |
| dcaliste | But it appeared in the log as a local deletion, which made me investigate. | 08:17 |
| dcaliste | But 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 |
| dcaliste | Actually, mKCal is already not applying alarms for deleted events. | 08:18 |
| dcaliste | So 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 |
| dcaliste | What do you think ? | 08:19 |
| chriadam | is this not already possible? | 08:19 |
| chriadam | we have something like this for exchange events, I think, at least? | 08:19 |
| dcaliste | In the RFC, cancelled events are adviced for organisers to push this to invitees. | 08:19 |
| dcaliste | I don't know, I'm not using Exchange :/ | 08:20 |
| dcaliste | So maybe, it would be better to keep the update for cancelled events instead of deleting them in CalDAV sync ? | 08:21 |
| chriadam | I wonder if flypig knows. I suspect there might be a participation status which specifies that the event was cancelled, perhaps | 08:21 |
| chriadam | I think you're right: deleting them altogether is probably unwanted | 08:21 |
| chriadam | the user can manually delete the cancelled event, if they choose. | 08:21 |
| flypig | Sorry, I've not been following, will try to catch up. | 08:21 |
| dcaliste | Ok, so I'll amend the commit to completely remove the deletion in case of cancelled events. | 08:22 |
| dcaliste | Hello flypig, it's about cancelled events. (STATUS: CANCELLED in iCal format). | 08:22 |
| dcaliste | What to do with them when upstream is applying this status. | 08:22 |
| dcaliste | At the moment, CalDAV is deleting them. And we're wondering how Exchange and Google are handling this. | 08:23 |
| chriadam | mostly I was wondering what we do in exchange plugin case. would be good to try to have same semantics. | 08:23 |
| dcaliste | And also (!), if there is a UI feedback already for cancelled events. | 08:23 |
| flypig | Okay, 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 |
| piggz | flypig: oh, forgot about Q's ... will do today! | 08:24 |
| dcaliste | Yeh, 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 |
| chriadam | thank you | 08:26 |
| chriadam | I 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 |
| flypig | For 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 |
| flypig | And I suspect the "Cancelled" prefix comes from the server changing the title, so probably we should be adding a local indication too. | 08:27 |
| chriadam | flypig: do we set partstat to cancelled or something? | 08:28 |
| dcaliste | Well, 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 |
| chriadam | i.e. is there a field which we can already use to show the indicator? | 08:29 |
| dcaliste | Ah, yeh, if the cancelled string is set by the server, that may be too peculiar to server which actually does it… | 08:29 |
| chriadam | sp/use to show/use in order to determine if we need to show, I meant | 08:29 |
| flypig | dcaliste, 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 |
| dcaliste | Looking at CalendarEvent class in nemo-qml-plugin-calendar, I'm surprised that the status is not exposed… | 08:31 |
| dcaliste | So, currently, chriadam, I would say that there is no way in QML to know that an event has been marked as cancelled. | 08:32 |
| chriadam | me too | 08:32 |
| chriadam | damn | 08:32 |
| flypig | It sounds like it wouldn't be hard to expose it. The info is retained internally. | 08:33 |
| dcaliste | So 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 |
| dcaliste | flypig: indeed, as for the rest, it needs to be duplicated in CalendarData::Event structure and then exposed in the Event object. | 08:34 |
| chriadam | dcaliste: 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 |
| flypig | It would also make sense to be consistent across EAS and CalDav. | 08:35 |
| chriadam | yes. | 08:35 |
| dcaliste | Right. 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 |
| chriadam | tyvm | 08:36 |
| chriadam | that would be greatly appreciated | 08:36 |
| flypig | Yeah, that sounds really nice. | 08:37 |
| chriadam | not sure when MartinS will get a chance to look into the design side, though | 08:37 |
| dcaliste | Prepending "cancelled" in subject in the sync plugin is a bit ugly though… Particularly if it requires localisation. :/ | 08:37 |
| chriadam | indeed, fair enough, let's avoid that for now | 08:37 |
| dcaliste | Another 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 |
| dcaliste | The 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 |
| dcaliste | The 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 |
| chriadam | the 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 |
| dcaliste | I 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 |
| chriadam | thank you very much for creating the PRs | 08:43 |
| chriadam | yes, those extra possibilities would be useful. requiring more work, though. This is a very good first step. | 08:44 |
| dcaliste | Indeed, I was hoping for some feedback beforz adding more options. | 08:44 |
| dcaliste | Technically they are not that complicated, it's just about setting more values to the flags and react accordingly in the delta calculation. | 08:45 |
| dcaliste | I'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 |
| dcaliste | That'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 |
| dcaliste | But I'm open to discussion, of course. | 08:47 |
| chriadam | it does seem like a natural plae for it. | 08:47 |
| chriadam | I agree. but, yeah, let's see what Joona and Martin suggest. | 08:48 |
| chriadam | you've been very busy these last few weeks - thank you very much, as always, for your efforts. | 08:49 |
| dcaliste | Yes, indeed, thank you. | 08:49 |
| chriadam | are there any other PRs which I need to put on my list to review? | 08:49 |
| dcaliste | And coming (almost) last, the infamous : https://github.com/sailfishos/sailfish-secrets/pull/182 | 08:50 |
| chriadam | oh, let me check | 08:50 |
| dcaliste | I'm a bit ashamed to see that I messed this up completely. | 08:50 |
| dcaliste | What I thought was a fix for some error I got with signature was a complete disaster : it killed the security-ui completely. | 08:51 |
| chriadam | haha no problem | 08:51 |
| chriadam | I will push to 4.4.x | 08:51 |
| dcaliste | I've no idea how I got the error first, and how I thought it was solved then with this. | 08:51 |
| chriadam | I thought I tested the original PR and it worked fine | 08:51 |
| chriadam | all good | 08:51 |
| chriadam | thanks | 08:51 |
| dcaliste | Restarting from scratch on 4.4.0.12 and recompiling secrets and ensuring that it's the right version running, this time it's working (with the revert). | 08:52 |
| dcaliste | So sorry for the loop. | 08:52 |
| chriadam | no problem at all | 08:53 |
| dcaliste | Thank you. | 08:53 |
| chriadam | give me a sec, triggering it now | 08:53 |
| dcaliste | After this one, there are just two minor doc updates : | 08:54 |
| dcaliste | - https://github.com/sailfishos/docs.sailfishos.org/pull/18 | 08:54 |
| dcaliste | - and https://github.com/sailfishos/docs.sailfishos.org/pull/19 | 08:54 |
| dcaliste | I noticed these while browsing the docs looking for something else. | 08:54 |
| dcaliste | It's quite complete actually. Thanks to sledges and those who made this possible. | 08:55 |
| chriadam | yes, it's really nice that it's on github now! | 08:57 |
| chriadam | thanks for those. I've approved one, and added a comment to the other | 08:57 |
| chriadam | thank you for those improvements | 08:57 |
| dcaliste | I've seen, I'll update it according to your suggestion. | 08:57 |
| chriadam | Unfortunately, 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 |
| chriadam | I will ping Pekka about that nqpc one which OMP need. | 08:58 |
| chriadam | and poke Martin and Joona about the jolla-calendar UI thing. | 08:58 |
| dcaliste | Yes, 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 |
| chriadam | :-) | 08:58 |
| chriadam | thanks - I hope you have a great week! | 08:59 |
| * chriadam -> away, gnight! | 08:59 | |
| dcaliste | Thank you, you too. | 08:59 |
| Thaodan | rinigus: Can you create python3-tomli-w for me in chum? | 09:09 |
| piggz | rinigus: yeah, feel free to look at the github api part | 09:52 |
| rinigus | Thaodan: later tonight, sure | 09:53 |
| rinigus | piggz: great, will do | 09:53 |
| Thaodan | rinigus: Thank's that was the last one :D | 09:54 |
| piggz | rinigus: 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 |
| piggz | other option would be something like findLastIndexOf("\n") ? | 09:57 |
| piggz | or probably second last? | 09:58 |
| piggz | no, would be \n\n for new line | 09:58 |
| piggz | s/blank line | 09:58 |
| Thaodan | piggz: which metadata? | 10:19 |
| Thaodan | ah just read it. Did you consider using appstream as metadata? | 10:20 |
| Thaodan | https://www.freedesktop.org/wiki/Distributions/AppStream/ | 10:20 |
| Thaodan | Only downside is that it's xml based | 10:21 |
| Thaodan | Qt backend also exists | 10:22 |
| piggz | Thaodan: yes, i asked rinigys about using appstream, and he wasnt in favour of it as it was a pita when used in flatpak iirc | 10:33 |
| piggz | this atlleast is quite simple for dev to add to a package | 10:33 |
| piggz | plus, how would we get the appstream data using nothing but an OBS repo | 10:38 |
| rinigus | piggz: 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 well | 11:00 |
| rinigus | Thaodan: as you asked in github as well, I'll better reply there. | 11:00 |
| rinigus | piggz: 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 description | 11:10 |
| Thaodan | piggz: From what I read they can be generated https://blogs.gnome.org/hughsie/2016/04/27/3rd-party-fedora-repositories-and-appstream/ | 11:10 |
| Keto | OBS seems to have some handling for the appdata.xml, but don't know if and how it works | 11:53 |
| Keto | but imho, that would seem like a saner option, if it works, than hacking something in the rpm descrioption | 11:54 |
| Thaodan | Well if hacking that into rpm would so easy it would have been done imho. | 13:24 |
| Thaodan | Keto: Do you know the require OBS version? | 13:28 |
| Keto | google gave some blogpost about obs 2.4 release which mentions appstream support, so it should be there | 13:36 |
| Keto | but I did not dig deeper | 13:37 |
| piggz | als lbt :D | 13:37 |
| Keto | afaik the community obs is 2.9 | 13:37 |
| rinigus | Thaodan: re "if hacking ... easy" comment - do you foresee some issues with the proposed approach? | 14:07 |
| Thaodan | rinigus: Duplication of metadata and character limit/encoding issue. The description field isn't made for what you want to do. | 14:08 |
| Thaodan | obs devs say running appstream over the rpm-md repo will create/extract any appstream metadata. | 14:09 |
| rinigus | Thaodan: 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 |
| Thaodan | Yes 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 |
| Thaodan | appstream 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 |
| rinigus | Thaodan: re pure maps appdata - that was required by flathub for pure maps distributed there. | 14:18 |
| rinigus | Thaodan: 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 |
| rinigus | Thaodan: 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 field | 14:22 |
| rinigus | Thaodan: char encoding - don't know much about encoding supported by SPEC. Is UTF8 supported? | 14:22 |
| Thaodan | OK 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 |
| Thaodan | rinigus: yes utf-8 works | 14:27 |
| Thaodan | https://github.com/hughsie/appstream-glib#appstream-builder | 14:27 |
| rinigus | Thaodan: 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 separately | 14:30 |
| rinigus | as 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 field | 14:32 |
| rinigus | but if you know SPEC char limits and encoding limitations, please tell | 14:32 |
| Thaodan | rinigus: 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 |
| Thaodan | If you want to know more ask #rpm.org on libera or github | 14:41 |
| rinigus | Thaodan: 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 |
| piggz | when i looked it up, rpm5 adds arbitrary fields, but we are on rpm4 | 14:45 |
| Thaodan | piggz: Yeah it did the same, a spec file is so nice to retrieve appdata from similar how to importlib.metadata | 14:46 |
| XenoPL | Hi, 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 |
| piggz | was hoping to discuss the issue with karry ... you here? | 14:49 |
| Thaodan | rinigus: Can you add python3-tomli-w to chum for me? | 16:58 |
| rinigus | Thaodan: was just pasting it into github form at the moment I got your ping :) | 16:59 |
| Thaodan | xD thanks | 16:59 |
| rinigus | Thaodan: done! | 17:00 |
| *** Mikaela is now known as Guest7889 | 21:15 | |
| piggz_ | rinigus: pushed branch to main repo, now parses last paragraph | 22:23 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!