From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:403:4ea1::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 8P2uCHUAy2HbaQAAgWs5BA (envelope-from ) for ; Tue, 28 Dec 2021 13:17:57 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id 2FebAnUAy2HrCQAAG6o9tA (envelope-from ) for ; Tue, 28 Dec 2021 13:17:57 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 8680C325FC for ; Tue, 28 Dec 2021 13:17:56 +0100 (CET) Received: from localhost ([::1]:49992 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n2BQQ-0005fe-5z for larch@yhetil.org; Tue, 28 Dec 2021 07:17:55 -0500 Received: from eggs.gnu.org ([209.51.188.92]:51498) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n2BKr-0000yC-SL for emacs-orgmode@gnu.org; Tue, 28 Dec 2021 07:12:09 -0500 Received: from wout5-smtp.messagingengine.com ([64.147.123.21]:42763) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n2BKl-0006Ax-TF for emacs-orgmode@gnu.org; Tue, 28 Dec 2021 07:12:06 -0500 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id E415332009CC for ; Tue, 28 Dec 2021 07:12:00 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Tue, 28 Dec 2021 07:12:01 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eml.cc; h= message-id:subject:from:to:in-reply-to:references:content-type :date:mime-version; s=fm2; bh=Az5bwIIWo/cpq0Jo7m1YSAbACTMtluFzXp tKv8p5On8=; b=JesvFfXwgEgYJHglgwaJwyU+4VIxeRQQJr86x9/arXzXupuTlA SJR6FYfQRsQ1cUl4hy2t56SJw92NVnyrxucFXhTj0aEJHfFOcBaeuKcncjDXJaNF i4gEcEqpXHuaafisPisN5sdMBIvKHZSYGYtM3Fh8o0kqfo71NBHAB/8f+gHoVxny SwguBDkmFCGhUe43n4cYlUKXN6O2ZptrnTLLLgdVFUuJ7Q/RcnxTFo1fSqN6Tbhr mcOPjzWZPIfOsuUwe6+PjhwGh6HJEQr+7nMYJC8mlJLHbJ+LYHyb4sGr9KmlGw8c JEH8wZr4+3kpUpjg43+lr8MWg70sO8XFBf9w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=Az5bwI IWo/cpq0Jo7m1YSAbACTMtluFzXptKv8p5On8=; b=ObOSw17knSdVX0fQzefT4Z JqJPevR0hkXO97cZ4n9aX845MD00ztTmPoH+sE6KPnQd/jBL7lXGYO8Ty3bEBW52 J0CJ9dNFoTAsZP+WwX/DH7Nx8DW4WLNvqsuJzxVHIPcuJiqQuXeLIQwFfEGbEjDN kieioi0e1xqfX/GFRY+sIiHUPJ6V1aPgsrDRPMG50MEERagnTUDSEyFP3K5drOA6 1OVDo4+qHVlLla6CIwQ7Ow6VsjsPBM5rojHqBcLxaVk03fasHTnaA7fQPzyUVD2a zI3IA2aYh3HGH6SkbfiUhi+3VGvQ8wpfXhmtFb/lqHwyj/p05EVC/6dZnRQXbK1A == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrudduledgfeekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne gfrhhlucfvnfffucdlfedtmdenucfjughrpefkuffhvfgjfhgtffggfgesmhdtreertder jeenucfhrhhomhepofhikhhhrghilhcuufhkohhriihhihhnshhkihhiuceomhhskhhorh iihhhinhhskhhihiesvghmlhdrtggtqeenucggtffrrghtthgvrhhnpeehteevledufeel ffeljedvgfekgffghfffteduffeghfeukeehtdeluddtudehleenucevlhhushhtvghruf hiiigvpedunecurfgrrhgrmhepmhgrihhlfhhrohhmpehmshhkohhriihhihhnshhkihih segvmhhlrdgttg X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Tue, 28 Dec 2021 07:11:59 -0500 (EST) Message-ID: <453f362a41c5d9350ce19273048e4808814c9123.camel@eml.cc> Subject: Re: [PATCH] ox-icalendar.el: create alarm at event time From: Mikhail Skorzhinskii To: Org Mode In-Reply-To: <875yrbawng.fsf@nicolasgoaziou.fr> References: <875yrbawng.fsf@nicolasgoaziou.fr> Content-Type: multipart/mixed; boundary="=-uoVQ+kJqLvM5bcVUKX9q" Date: Tue, 28 Dec 2021 13:08:40 +0100 MIME-Version: 1.0 User-Agent: Evolution 3.40.4-1 Received-SPF: pass client-ip=64.147.123.21; envelope-from=mskorzhinskiy@eml.cc; helo=wout5-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1640693876; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=Az5bwIIWo/cpq0Jo7m1YSAbACTMtluFzXptKv8p5On8=; b=b9BA7w+F2ROF+IWt3P/E0Qhb84npKQ3kCD3WvNKBfvLNJRziSS1cM2cb30SH8A3gCcb3Hw uI5yyuOBi+XHyUOaKlseW4XgZRAwIM9RgB+8EkDy5g9YhGit1odnr72DYCVAEPpiOKf5JV wpmXJJGI/rJOfSjYW3og8XDqJN+ErtW2SFYnLv+NgXYkeXaZMPVt1/+RWLnCvxvJ2jQy7X Dsbiim9D+96XLcqJmK0uTeCnmAiKFeYnxr614xBh0BFZbH/BU9z4OKtEdXiLPZqgP6yTTu YhK8DJ8o0vm1F4/VB9ZJ164MpeUyhYvSJebVDqKSSiAoaj6vIzBRCZj50F2WYw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1640693876; a=rsa-sha256; cv=none; b=BrEHjUdfTht1lUQmnwxw8qwJD5wGej6FdL6OJE9e/jdBXkrCWjmWlhZpnr3L6Ug9tYY8sG tw/asvNL5DTlZQB8AJdb7+ZebiWVInZgvS5+ZqwQhX+er3MsCXlZXmQb0m9kOwycU60MYA 0gPq5v921bPeJKuSMOvjcICXTHaYmhR7SOhNsnlSyS6EgkAOrTlrU4d7fvIorvMceyIN7I 47w/h4sLyaJZ5y6K+Iasj7Oh8nl50Zt2G7oqSiXe57VtBnJHVZYPECJ0p3czilDQijzgEW Cf2hiRGNQZ4F5FCY0oQg7c6er2HEf2VBe8fa3WIpqGhXSAWlkMSkfw7SiWWcZQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=eml.cc header.s=fm2 header.b=JesvFfXw; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=ObOSw17k; dmarc=pass (policy=none) header.from=eml.cc; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -4.27 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=eml.cc header.s=fm2 header.b=JesvFfXw; dkim=pass header.d=messagingengine.com header.s=fm1 header.b=ObOSw17k; dmarc=pass (policy=none) header.from=eml.cc; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 8680C325FC X-Spam-Score: -4.27 X-Migadu-Scanner: scn1.migadu.com X-TUID: +u0Vax8jaDYJ --=-uoVQ+kJqLvM5bcVUKX9q Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Hi Nicolas, Thank you for the review, appreciate your comments. I've applied your suggestions. Please find the fixed file in the attachment to this letter. There is one important note that about this patch. There is one corner case: when we've set org-icalendar-force-alarm variable to nil, org- icalendar-alarm-time variable to non-nil and the APPT_WARNTIME property is set to 0, then, with my code, the value of the org-icalendar-alarm- time would be used. Basically if user is not forcing the 'zero alarms', and has some non- zero default value for alarms, the APPT_WARNTIME property will use default alarm. Which is something, I would say, unexpected. I would expect that alarm will be shut-off. However I am just keeping the previous behaviour. I think its worth fixing, but I'd say we do this in a separate patch. Let me know what you think. Thanks, Mikhail On Sun, 2021-12-26 at 22:22 +0100, Nicolas Goaziou wrote: > Hello, > > Mikhail Skorzhinskii writes: > > > * lisp/ox-icalendar.el (org-icalendar-force-alarm): option to set > > alarm > > even if alarm time is set to zero. > > * lisp/ox-icalendar.el (org-icalendar--valarm): create VALARM at > > the > > event start if the alarm time is set to zero and > > `org-icalendar-force-alarm' is set to true. > > Thanks. Some comments follow. > > > +(defcustom org-icalendar-force-alarm nil > > +  "Non-nil means alarm will be created even if is set to zero. > > + > > +This overrides default behaviour where zero means no alarm. With >                                                              ^^^ > You need two spaces after full stop. > > > +this set to non-nil and alarm set to zero, alarm will be created > > +and will fire at the event start." > > +  :group 'org-export-icalendar > > +  :type 'bool) > > `boolean' is the valid type. > > You also need to add :package-version '(Org . "9.6") and :safe > #'booleanp. > > +                (if org-icalendar-force-alarm > > +                    (if alarm-time > > +                        alarm-time > > +                      org-icalendar-alarm-time) > > +                  (if (zerop alarm-time) > > +                      org-icalendar-alarm-time > > +                    alarm-time)))))) > > I suggest to refactor the above into something like: > > (cond >  ((> alarm-time 0) alarm-time) >  ((and (= 0 alarm-time) org-icalendar-force-alarm) alarm-time) >  (t org-icalendar-alarm-time)) > > Could you send an updated patch? > > Regards, > -- > Nicolas Goaziou --=-uoVQ+kJqLvM5bcVUKX9q Content-Disposition: attachment; filename="0004-ox-icalendar.el-create-alarm-at-event-time.patch" Content-Transfer-Encoding: base64 Content-Type: text/x-patch; name="0004-ox-icalendar.el-create-alarm-at-event-time.patch"; charset="UTF-8" RnJvbSAzOWIwZGYzMzA5NjA3ZjYxZDEwODQwMjc0OGQ2NjQ2OTM5Zjk4Njk2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNaWtoYWlsIFNrb3J6aGluc2tpaSA8bXNrb3J6aGluc2tpeUBl bWwuY2M+CkRhdGU6IFNhdCwgMTIgU2VwIDIwMjAgMTg6NTI6MzkgKzAyMDAKU3ViamVjdDogW1BB VENIIDQvNV0gb3gtaWNhbGVuZGFyLmVsOiBjcmVhdGUgYWxhcm0gYXQgZXZlbnQgdGltZQoKKiBs aXNwL294LWljYWxlbmRhci5lbCAob3JnLWljYWxlbmRhci1mb3JjZS1hbGFybSk6IG9wdGlvbiB0 byBzZXQgYWxhcm0KZXZlbiBpZiBhbGFybSB0aW1lIGlzIHNldCB0byB6ZXJvLgoqIGxpc3Avb3gt aWNhbGVuZGFyLmVsIChvcmctaWNhbGVuZGFyLS12YWxhcm0pOiBjcmVhdGUgVkFMQVJNIGF0IHRo ZQpldmVudCBzdGFydCBpZiB0aGUgYWxhcm0gdGltZSBpcyBzZXQgdG8gemVybyBhbmQKYG9yZy1p Y2FsZW5kYXItZm9yY2UtYWxhcm0nIGlzIHNldCB0byB0cnVlLgotLS0KIGxpc3Avb3gtaWNhbGVu ZGFyLmVsIHwgMjQgKysrKysrKysrKysrKysrKysrKystLS0tCiAxIGZpbGUgY2hhbmdlZCwgMjAg aW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9saXNwL294LWljYWxl bmRhci5lbCBiL2xpc3Avb3gtaWNhbGVuZGFyLmVsCmluZGV4IDE4OWUzNTk0Ni4uMTg3MGE3MmI4 IDEwMDY0NAotLS0gYS9saXNwL294LWljYWxlbmRhci5lbAorKysgYi9saXNwL294LWljYWxlbmRh ci5lbApAQCAtNjYsNiArNjYsMTcgQEAgZm9yIHRpbWVkIGV2ZW50cy4gIElmIG5vbi16ZXJvLCBh bGFybXMgYXJlIGNyZWF0ZWQuCiAgIDp2ZXJzaW9uICIyNC4xIgogICA6dHlwZSAnaW50ZWdlcikK IAorKGRlZmN1c3RvbSBvcmctaWNhbGVuZGFyLWZvcmNlLWFsYXJtIG5pbAorICAiTm9uLW5pbCBt ZWFucyBhbGFybSB3aWxsIGJlIGNyZWF0ZWQgZXZlbiBpZiBpcyBzZXQgdG8gemVyby4KKworVGhp cyBvdmVycmlkZXMgZGVmYXVsdCBiZWhhdmlvdXIgd2hlcmUgemVybyBtZWFucyBubyBhbGFybS4g IFdpdGgKK3RoaXMgc2V0IHRvIG5vbi1uaWwgYW5kIGFsYXJtIHNldCB0byB6ZXJvLCBhbGFybSB3 aWxsIGJlIGNyZWF0ZWQKK2FuZCB3aWxsIGZpcmUgYXQgdGhlIGV2ZW50IHN0YXJ0LiIKKyAgOmdy b3VwICdvcmctZXhwb3J0LWljYWxlbmRhcgorICA6dHlwZSAnYm9vbGVhbgorICA6cGFja2FnZS12 ZXJzaW9uICcoT3JnIC4gIjkuNiIpCisgIDpzYWZlICMnYm9vbGVhbnApCisKIChkZWZjdXN0b20g b3JnLWljYWxlbmRhci1jb21iaW5lZC1uYW1lICJPcmdNb2RlIgogICAiQ2FsZW5kYXIgbmFtZSBm b3IgdGhlIGNvbWJpbmVkIGlDYWxlbmRhciByZXByZXNlbnRpbmcgYWxsIGFnZW5kYSBmaWxlcy4i CiAgIDpncm91cCAnb3JnLWV4cG9ydC1pY2FsZW5kYXIKQEAgLTgwNyw4ICs4MTgsMTEgQEAgUmV0 dXJuIFZBTEFSTSBjb21wb25lbnQgYXMgYSBzdHJpbmcsIG9yIG5pbCBpZiBpdCBpc24ndCBhbGxv d2VkLiIKICAgKGxldCAoKGFsYXJtLXRpbWUKIAkgKGxldCAoKHdhcm50aW1lCiAJCShvcmctZWxl bWVudC1wcm9wZXJ0eSA6QVBQVF9XQVJOVElNRSBlbnRyeSkpKQotCSAgIChpZiB3YXJudGltZSAo c3RyaW5nLXRvLW51bWJlciB3YXJudGltZSkgMCkpKSkKLSAgICAoYW5kIChvciAoPiBhbGFybS10 aW1lIDApICg+IG9yZy1pY2FsZW5kYXItYWxhcm0tdGltZSAwKSkKKwkgICAoaWYgd2FybnRpbWUg KHN0cmluZy10by1udW1iZXIgd2FybnRpbWUpIG5pbCkpKSkKKyAgICAoYW5kIChvciAoYW5kIGFs YXJtLXRpbWUKKwkJICAoPiBhbGFybS10aW1lIDApKQorCSAgICAgKD4gb3JnLWljYWxlbmRhci1h bGFybS10aW1lIDApCisJICAgICBvcmctaWNhbGVuZGFyLWZvcmNlLWFsYXJtKQogCSAob3JnLWVs ZW1lbnQtcHJvcGVydHkgOmhvdXItc3RhcnQgdGltZXN0YW1wKQogCSAoZm9ybWF0ICJCRUdJTjpW QUxBUk0KIEFDVElPTjpESVNQTEFZCkBAIC04MTYsOCArODMwLDEwIEBAIERFU0NSSVBUSU9OOiVz CiBUUklHR0VSOi1QMERUMEglZE0wUwogRU5EOlZBTEFSTVxuIgogCQkgc3VtbWFyeQotCQkgKGlm ICh6ZXJvcCBhbGFybS10aW1lKSBvcmctaWNhbGVuZGFyLWFsYXJtLXRpbWUgYWxhcm0tdGltZSkp KSkpCi0KKyAgICAgICAgICAgICAgICAgKGNvbmQKKyAgICAgICAgICAgICAgICAgICgoYW5kIGFs YXJtLXRpbWUgb3JnLWljYWxlbmRhci1mb3JjZS1hbGFybSkgYWxhcm0tdGltZSkKKyAgICAgICAg ICAgICAgICAgICgoYW5kIGFsYXJtLXRpbWUgKG5vdCAoemVyb3AgYWxhcm0tdGltZSkpKSBhbGFy bS10aW1lKQorICAgICAgICAgICAgICAgICAgKHQgb3JnLWljYWxlbmRhci1hbGFybS10aW1lKSkp KSkpCiAKIDs7OzsgVGVtcGxhdGUKIAotLSAKMi4zMi4wCgo= --=-uoVQ+kJqLvM5bcVUKX9q--