From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id ODozGd1VUmIGOwEAgWs5BA (envelope-from ) for ; Sun, 10 Apr 2022 05:58:21 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id 0APWFN1VUmLJjAAAauVa8A (envelope-from ) for ; Sun, 10 Apr 2022 05:58:21 +0200 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 D0D961F7F4 for ; Sun, 10 Apr 2022 05:58:20 +0200 (CEST) Received: from localhost ([::1]:46184 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ndOiR-0006s4-4M for larch@yhetil.org; Sat, 09 Apr 2022 23:58:19 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57436) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ndOhe-0006rk-04 for emacs-orgmode@gnu.org; Sat, 09 Apr 2022 23:57:30 -0400 Received: from mail-lf1-x132.google.com ([2a00:1450:4864:20::132]:45702) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ndOhb-0003v7-Q4 for emacs-orgmode@gnu.org; Sat, 09 Apr 2022 23:57:29 -0400 Received: by mail-lf1-x132.google.com with SMTP id p10so21138778lfa.12 for ; Sat, 09 Apr 2022 20:57:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:message-id:date:mime-version:user-agent:from:subject:to :references:content-language:in-reply-to:content-transfer-encoding; bh=FGP5jC++ziOkZY+copf+02mvaWB0g3YaI1wS9+knCHo=; b=SiJoLAFgGD5i5MjMarKmuYFNgD1KGKLqOtpLb98oebQehX3y2x1kjreNNI5NlTB65y yeQDHwI50afYRJywvhs7UH8x8XbDfaJPvFUh7kUMvJbxRb2aMseI6lJuHXIgRNgwN/Jq 2ptC6FjYY5yMji5uUGXFQ5IJsMia1q8O+5DRcT6ruVnM0H7m4ptOUx3/frmFrGixMECE zqGyNqgYbhIjzHPOs1v55cGIAIzuHDh63meoF4bLVzXOB46AscKJMC+7nvEWLsgzTqSx c1x2iOHZkc2WlTef3Q5kfHpUrvevC89tYX4uRspDjyyWx46SFKXVD/mEA9A/K6DmBsOt mDgA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:message-id:date:mime-version:user-agent :from:subject:to:references:content-language:in-reply-to :content-transfer-encoding; bh=FGP5jC++ziOkZY+copf+02mvaWB0g3YaI1wS9+knCHo=; b=ehfpdkuvdPvPHbq6RxPqV65QPM26IVztGtCCLCeYvgoyEKnPHEAiVSzb04RnqUA6Th UAWmVfJxDTJSmC/YaWWoDDqe2nwKKgVW2yW0O82ZyzRB2DE6M/I7GEQ1T3Smqtv9xfE9 UVpcVdnnnDkzBc6glkMPaHBlR5DU4wUSBdriPlC5coOmbiCTBD/QMPvq2nBMrfH8JdbD 7oAGPmsd4kyB8i7LbtqnYNfduGbz3K7rWcajoARJgsc0TI1C5Xu+53X/WV4TS8AJlUHN 4D715bdCPhtVufYRLWiKJH21ca958JjHJnL2TBV63f4RBtPpgSB/+h1sCjz3BZg4zR51 VFrw== X-Gm-Message-State: AOAM530LeyvpuWR65Vdl/YSy5E5EJFrkt0C5B49irokCeN7TPRgKeerY 2POOyaHBc6X2UI7TwsMm1lVI0HD0z6I= X-Google-Smtp-Source: ABdhPJxS6KrIst/Vjo9sh2bHQmEksaRY+r/NlYc7XF2PaCWbplWr/376+N5gfmukeIHMY4vp+EnAhg== X-Received: by 2002:a05:6512:2251:b0:44a:9707:b54e with SMTP id i17-20020a056512225100b0044a9707b54emr17346052lfu.189.1649563044843; Sat, 09 Apr 2022 20:57:24 -0700 (PDT) Received: from [192.168.0.101] (nat-0-0.nsk.sibset.net. [5.44.169.188]) by smtp.googlemail.com with ESMTPSA id p18-20020a19f112000000b0046b9e4242e5sm161052lfh.191.2022.04.09.20.57.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 09 Apr 2022 20:57:24 -0700 (PDT) Message-ID: <6b323915-14f9-b049-db01-ceb358ee693f@gmail.com> Date: Sun, 10 Apr 2022 10:57:21 +0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 From: Max Nikulin Subject: Re: bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones To: orgmode , Paul Eggert References: <5ed963b2-3fa8-48d8-627e-bc0571d15b43@gmail.com> <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> Content-Language: en-US In-Reply-To: <149de00f-115b-5367-414f-c7700ef8966b@cs.ucla.edu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a00:1450:4864:20::132; envelope-from=manikulin@gmail.com; helo=mail-lf1-x132.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 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, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1649563101; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=FGP5jC++ziOkZY+copf+02mvaWB0g3YaI1wS9+knCHo=; b=fCvaPQk/AJH9DVLarj3TDHbaErkXrew06iL35x0PsoPxTEPuhcd1N0YazLP4+eNX/L0f0x eN8NZp+QHtzN/wsLF4cXDKzxDuDdGaFzXMSSG+TSYGfAEzsKfcalERhOb3Lv1Ih9hZ2W/3 Ixn7LI+DARMHyJghXlLVlOwFVND8xlmy1rXmelw7tyh9EQQd2RN+csDhi5Bfm/iwYdwy/j veFNnqfkOcjK2TiwDcq6WskTroB5lVv9+EiQXRGcqatA+446+EkdHZTB9TDQEwRzZPkLkU ejWJMOqxz5mEdh4A3rGP4eebS0H4Cid9WYc/41yR9rZNVtEUchuBFHfjX8kAWA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1649563101; a=rsa-sha256; cv=none; b=iqnWHjy31+i5AMZgaw+ZAJpSByxm0Kh4908kxaeQBhROMdtSub2QE67aYwTxBVy7DyG+is r0yDD9Mr8zlYq1Opj5jiP+kuw3uVZNZFgpDfqO0dEd8QpqXnqxb3mA/0xBxvJgR8T09CtK K3OYJALeTmv/97LRl9Ty35imib23gIhvFpruoVy/GKHZpF3JQ4bod0BJcZnaFOvy4ZIaMn y5UqWEPgQaLEt4s0rPIrdS3ZUi7OTY0fw2cRBv9NzdFgkNOYbRpyt12SJo3vJvjlkTBsDp 2Gx5scQbSGHIEi1nhoht2KxXcRHTeQztOqbG7NlMVLFlGz2ZnKKej4uJVOhIrQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=SiJoLAFg; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); 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: 6.33 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=SiJoLAFg; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); 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: D0D961F7F4 X-Spam-Score: 6.33 X-Migadu-Scanner: scn0.migadu.com X-TUID: wNMRH6cLW/tk I am sorry, I have realized that debbugs.gnu.org does not send copies to the addresses specified in the X-Debbugs-CC header of the original report. Now I am sending a copy of my earlier message to emacs-orgmode. Please, add 54764@debbugs.gnu.org to replies to this message. Paul, I am sorry that I forced you to make more changes in the Org code. While I was filing this bug, my intention was to add something like "if (!NILP(...)) { .... }" around several lines in src/timefns.c. Now we should decide whether we leave this bug for possible changes in the `encode-time' implementation and moving the discussion related to further changes in Org to the emacs-orgmode mail list or we change the title of this bug and maybe create another one for `encode-time'. On 09/04/2022 14:52, Paul Eggert wrote: > On 4/7/22 05:37, Max Nikulin wrote: > >>      (encode-time '(0 30 20 07 04 2022 nil nil nil)) ; wrong! > > Yes, and I see a couple of places (org-parse-time-string, > org-read-date-analyze) where Org mode returns the wrong decoded > timestamps, ending in (nil nil nil) I see you even change `org-store-link' that has the same problem. > Obsolescent, not obsolete. The old form still works and it's not going > away any time soon. If the efficiency concerns you mention are > significant, we should keep the old form indefinitely. I am against preserving the old form because it ignores the DST field. It is confusing and error prone to be a part of a well designed interface. Actually I have a draft of `org-encode-time' macro that transforms 6 elements list to 9 elements one at load or compile time, so it should not hurt runtime performance. I have not tried to replace all calls of the `encode-time' function yet however. But I still prefer to drop 6/9 elements branch even if it may happen a decade later. >>  From my point of view it is better to change implementation of >> `encode-time' so that it may accept 6-component list SECOND...YEAR. It >> should not add noticeable performance penalty but makes the function >> more convenient in use. > > Unfortunately it makes the function more convenient to use incorrectly. > This was part of the motivation for the API change. The obsolescent > calling convention has no way to deal with ambiguous timestamps like > 2022-11-06 01:30 when TZ="America/Los_Angeles". Org mode surely has bugs > in this area, although I don't have time to scout them out. I do not see your point here. Old calling convention did not allow to specify DST flag at all and it was a problem. With the list argument even if last 3 fields are optional, it will not prevent adding DST value when it is known from some source. I think, requiring 3 extra fields when DST value is unknown is too hing price just to make a developer aware that it may be ambiguous (moreover it will not work without a clear statement in the docstring). Org timestamps does not allow to specify timezone abbreviation such as PDT/PST to distinguish DST. If it were added then users would have false impression of full time zones support in Org. It would require huge amount of work. So guessed DST is the best that often can be offered. >> Daylight saving time field matters only as a list component and >> ignored as a separate argument (by the way, it should be stressed in >> the docstring). > > Do you have a wording suggestion? (The doc string already covers the > topic concisely; however, conciseness is not always a virtue. :-) My point is that subtle breaking changes must be prominent and hard to ignore. I do not have yet ready phases and you highlighted another issue missed in the docstring. >> So my proposal is to not force Org mode to use new calling convention >> for `encode-time' till DST and ZONE list components will became >> optional ones in a released Emacs version. > > This would delay things for ten years or so, no? We'd have to wait until > Org mode supported only Emacs 29 and later. I do not think it is a problem. > Instead, I suggest that we stick with what we have when that's cleaner. > That is, Org mode can use the obsolescent encode-time API when it's > cleaner to do that. I considered such approach to defense against "aggressive" modernizing of Emacs code. Then I decided it is better to allow to deprecate one of the styles of calling `encode-time'. I tried to express it in the original report and above. > I haven't installed the patch, or tested it other than via 'make check'. Org has its own repository and changes should be committed there at first. Org has unit tests, see https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/testing/README you changes should be accompanied by some adjustments in test expectations. I believe that at least in some cases Org test suite should be run for the bundled version of Org after Emacs builds. There are may be copyright issues with including the tests into the Emacs source tree. Maybe some changes in tests were accepted without paperwork. > PS. Org mode usually uses encode-time for calendrical calculations. This > is dicey, as some days don't exist (for example, December 30, 2011 does > not exist if TZ="Pacific/Apia", because Samoa moved across the > International Date Line that day). And it's also dicey when Org mode > uses 00:00:00 (midnight at the start of the day) as a timestamp > representing the entire day, as it's all too common for midnight to not > exist (or to be duplicated) due to a DST transition. Generally speaking, > when Org mode is doing calendrical calculations it should use > calendrical functions rather than encode-time+decode-time, which are > best used for time calculations not calendar calculations. (I realize > that fixing this in Org would be nontrivial; perhaps I should file this > "PS" as an Org bug report for whoever has time to fix it....) I agree with you that dates should not be represented as timestamps and date modifications (day, week, month, year increments and decrements) should be performed by dedicated functions. I even had 2011-12-30 example in my notes. However I expect that underlying normalization of date-time fields may mitigate such issues to some extent. > diff --git a/lisp/org/org-compat.el b/lisp/org/org-compat.el > index 819ce74d93..247373d6b9 100644 > --- a/lisp/org/org-compat.el > +++ b/lisp/org/org-compat.el > @@ -115,6 +115,27 @@ org-table1-hline-regexp > (defun org-time-convert-to-list (time) > (seconds-to-time (float-time time)))) > > +;; Like Emacs 27+ `encode-time' with one argument. > +(if (ignore-errors (encode-time (decode-time))) > + (defsubst org-encode-time-1 (time) > + (encode-time time)) > + (defun org-encode-time-1 (time) > + (let ((dst-zone (nthcdr 7 time))) > + (unless (consp (cdr dst-zone)) > + (signal wrong-type-argument (list time))) > + (let ((etime (apply #'encode-time time)) > + (dst (car dst-zone)) > + (zone (cadr dst-zone))) > + (when (and (symbolp dst) (not (integerp zone)) (not (consp zone))) > + (let* ((detime (decode-time etime)) > + (dedst (nth 7 detime))) > + (when (and (not (eq dedst dst)) (symbolp dedst)) > + ;; Assume one-hour DST and adjust the timestamp. > + (setq etime (time-add etime (seconds-to-time > + (- (if dedst 3600 0) > + (if dst 3600 0)))))))) > + etime)))) > + > ;; `newline-and-indent' did not take a numeric argument before 27.1. > (if (version< emacs-version "27") > (defsubst org-newline-and-indent (&optional _arg) > diff --git a/lisp/org/org-macro.el b/lisp/org/org-macro.el > index 0921f3aa27..b8e4346002 100644 I do not have complicated agenda setup to profile performance after such change. I will post my simple macro when we decide to continue discussion on debbugs or in a dedicated thread of the emacs-orgmode list. In my opinion, the code obtaining DST value requires unit tests. I like you idea to reuse `org-time-string-to-seconds' in more places. My original plan was to use `org-time-string-to-time' there.