emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Leap-year bug with todo-cycle
@ 2024-03-29 10:48 Anton Haglund
  2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Anton Haglund @ 2024-03-29 10:48 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

Hi!

I think I have discovered a possible leap-year bug in todo-cycle.

I have a TODO-entry which looks like this:

SCHEDULED: <2024-02-29 Thu ++1y>

When I cycle the TODO-entry with c-c c-t it becomes

SCHEDULED: <2025-03-01 Sat ++1y>

In my opinion it should become "2025-02-28 Fri" instead. If you don't
agree please give me the reasoning behind this.

Best regards,
Anton Haglund


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle)
  2024-03-29 10:48 Leap-year bug with todo-cycle Anton Haglund
@ 2024-04-05 18:34 ` Ihor Radchenko
  2024-04-05 19:53   ` Russell Adams
                     ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ihor Radchenko @ 2024-04-05 18:34 UTC (permalink / raw)
  To: Anton Haglund; +Cc: emacs-orgmode

Anton Haglund <aoh@lysator.liu.se> writes:

> I think I have discovered a possible leap-year bug in todo-cycle.
>
> I have a TODO-entry which looks like this:
>
> SCHEDULED: <2024-02-29 Thu ++1y>
>
> When I cycle the TODO-entry with c-c c-t it becomes
>
> SCHEDULED: <2025-03-01 Sat ++1y>

This is expected. When we try to add 1 year to 2024-02-29, it is
2025-02-29. However, because 02-29 does not exist in 2025, we glibc
takes the closest existing date and adds the difference in days:
2025-02-28 + 1d = 2025-03-01.

We apply the same logic to +1m repeaters:

SCHEDULED: <2024-05-31 Fri ++1m>
will become
SCHEDULED: <2024-07-01 Mon ++1m>
since 2024-06-31 does not exist.

> In my opinion it should become "2025-02-28 Fri" instead.

Keeping "end of a month" being end of a month is indeed an alternative
approach in such a situation. Both are possible; we just use the one
that is easier to implement from technical perspective.

Generally, I did see several requests to change the strategy when
calculating next month/year. However, that would be a breaking change.
I'd only go for it if people are strongly in favor of the change.
So, changing this to a poll.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle)
  2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
@ 2024-04-05 19:53   ` Russell Adams
  2024-04-05 21:18   ` jman
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Russell Adams @ 2024-04-05 19:53 UTC (permalink / raw)
  To: emacs-orgmode

On Fri, Apr 05, 2024 at 06:34:29PM +0000, Ihor Radchenko wrote:
> > In my opinion it should become "2025-02-28 Fri" instead.
>
> Keeping "end of a month" being end of a month is indeed an alternative
> approach in such a situation. Both are possible; we just use the one
> that is easier to implement from technical perspective.

So the poll is asking:

 If leap date doesn't exist, round date down instead of up?

I think that makes sense, at least it stays the same month.

+1

Thanks.

------------------------------------------------------------------
Russell Adams                            RLAdams@AdamsInfoServ.com
                                    https://www.adamsinfoserv.com/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle)
  2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
  2024-04-05 19:53   ` Russell Adams
@ 2024-04-05 21:18   ` jman
  2024-04-05 21:27     ` Ihor Radchenko
  2024-04-06 14:52   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
  2024-05-13 10:07   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
  3 siblings, 1 reply; 12+ messages in thread
From: jman @ 2024-04-05 21:18 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: Anton Haglund, emacs-orgmode


> Generally, I did see several requests to change the strategy when
> calculating next month/year. However, that would be a breaking change.
> I'd only go for it if people are strongly in favor of the change.
> So, changing this to a poll.

I don't particularly have a skin in the game but I ask a question: what would be the impact of this breaking change for users with existing Orgmode documents?

thanks


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle)
  2024-04-05 21:18   ` jman
@ 2024-04-05 21:27     ` Ihor Radchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Ihor Radchenko @ 2024-04-05 21:27 UTC (permalink / raw)
  To: jman; +Cc: Anton Haglund, emacs-orgmode

jman <emacs-orgmode@city17.xyz> writes:

> I don't particularly have a skin in the game but I ask a question: what would be the impact of this breaking change for users with existing Orgmode documents?

Mostly that people familiar with the current behaviour might be
surprised.

I am personally slightly in favour of the change, and I do not see
obvious downsides other than a surprise, but Org mode is used by many
people with various, sometimes surprising, workflows...

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
  2024-04-05 19:53   ` Russell Adams
  2024-04-05 21:18   ` jman
@ 2024-04-06 14:52   ` Max Nikulin
  2024-04-07 11:47     ` Ihor Radchenko
  2024-05-13 10:07   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
  3 siblings, 1 reply; 12+ messages in thread
From: Max Nikulin @ 2024-04-06 14:52 UTC (permalink / raw)
  To: emacs-orgmode

On 06/04/2024 01:34, Ihor Radchenko wrote:
> Generally, I did see several requests to change the strategy when
> calculating next month/year. However, that would be a breaking change.
> I'd only go for it if people are strongly in favor of the change.
> So, changing this to a poll.

I think the following should be taken into account: behavior of popular 
calendar applications, specifications they implement, libraries that 
likely used to create such applications.

Should it be configurable per user, per file, or even per rule 
(timestamp with repeater)?

I am not familiar with RFC5545 iCalendar, so I am unsure what options it 
recommends.

An example of recently designed library (however Org does not support 
calendars with leap *months*):
<https://tc39.es/proposal-temporal/docs/calendar.html#handling-unusual-dates-leap-days-leap-months-and-skipped-or-repeated-periods>
"Handling unusual dates: leap days, leap months, and skipped or repeated 
periods" (Temporal proposal for JavaScript)

> When Temporal encounters inputs representing a month and/or day that
> doesn't exist in the desired calendar year, by default (overridable in
> with or from via the overflow option) the inputs will be adjusted using
> the following algorithm:
> 
> - First, pick the closest day in the same month. If there are two
> equally-close dates in that month, pick the later one.
> - If the month is a leap month that doesn't exist in the desired year,
> then pick another date according to the cultural conventions of that
> calendar's users. Usually this will result in the same day in the month
> before or the month after where that month would normally fall in a leap
> year.
> - Otherwise, pick the closest date to the provided date that is still in
> the same year. If there are two equally-close dates, pick the later one.
> - If the entire year doesn't exist, then pick the closest date to the
> provided date. If there are two equally-close dates, pick the later one.




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-04-06 14:52   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
@ 2024-04-07 11:47     ` Ihor Radchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Ihor Radchenko @ 2024-04-07 11:47 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:

> I think the following should be taken into account: behavior of popular 
> calendar applications, specifications they implement, libraries that 
> likely used to create such applications.

### what happens in google calendar if you repeat task monthly at 31st day of month and the next month has fewer days?

ChatGPT:

If you set a task to repeat monthly on the 31st day of the month in Google Calendar, and the next month has fewer days (e.g., February or some months with 30 days), the task will fall on the last day of that month.

### 

It is the same as the change proposed in the POLL.

> Should it be configurable per user, per file, or even per rule 
> (timestamp with repeater)?

We ought to have a switch to the existing behaviour, but I do not think
that we need it per-file or per-timestamp. If we do need such
granularity, it implies that some people actually prefer the existing
behaviour - something we are yet to hear in the replies to this poll.

> An example of recently designed library (however Org does not support 
> calendars with leap *months*):
> <https://tc39.es/proposal-temporal/docs/calendar.html#handling-unusual-dates-leap-days-leap-months-and-skipped-or-repeated-periods>
> "Handling unusual dates: leap days, leap months, and skipped or repeated 
> periods" (Temporal proposal for JavaScript)

This also picks the closest existing date - the change we propose in
this POLL.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle)
  2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
                     ` (2 preceding siblings ...)
  2024-04-06 14:52   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
@ 2024-05-13 10:07   ` Ihor Radchenko
  2024-05-14 11:08     ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
  3 siblings, 1 reply; 12+ messages in thread
From: Ihor Radchenko @ 2024-05-13 10:07 UTC (permalink / raw)
  To: Anton Haglund; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:

>> I have a TODO-entry which looks like this:
>>
>> SCHEDULED: <2024-02-29 Thu ++1y>
>>
>> When I cycle the TODO-entry with c-c c-t it becomes
>>
>> SCHEDULED: <2025-03-01 Sat ++1y>
>
> This is expected. When we try to add 1 year to 2024-02-29, it is
> 2025-02-29. However, because 02-29 does not exist in 2025, we glibc
> takes the closest existing date and adds the difference in days:
> 2025-02-28 + 1d = 2025-03-01.
>
> We apply the same logic to +1m repeaters:
>
> SCHEDULED: <2024-05-31 Fri ++1m>
> will become
> SCHEDULED: <2024-07-01 Mon ++1m>
> since 2024-06-31 does not exist.
>
>> In my opinion it should become "2025-02-28 Fri" instead.

Given the positive responses on changing the date rounding, I went ahead
and tried to implement it (see the attached; note that some tests still
need to be fixed to address the below divergence in edge cases).

However, there are still some issues remaining.
When updating timestamps repeating monthly across months with 30, 31,
and 28 days we get 

<2025-01-31 Fri +1m>
<2025-02-28 Fri +1m>
<2025-03-28 Fri +1m>
...
<2026-01-28 Wed +1m>

As you can see, because we pass through February that only has 28 days,
the timestamp tends to drift towards 28th within one year.

With the existing approach the drift would not be much better though:

<2025-01-31 Fri +1m>
<2025-03-03 Mon +1m>
<2025-03-03 Mon +1m>
...
<2026-01-03 Sat +1m>

I am wondering if we should do something with this kind of edge case.
(Not that the proposed patch is going to make things worse, but maybe
you have some ideas on what can be done, while we are at it)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-m-y-repeater-intervals-round-down-from-non-exis.patch --]
[-- Type: text/x-patch, Size: 10204 bytes --]

From 99e4d3c0afd438499ab55314d30a01da54b15d53 Mon Sep 17 00:00:00 2001
Message-ID: <99e4d3c0afd438499ab55314d30a01da54b15d53.1715594311.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Mon, 13 May 2024 11:36:09 +0300
Subject: [PATCH] Make m/y repeater intervals round down from non-existing
 calendar dates

* lisp/org.el (org-repeat-round-time): New customization controlling
the new behavior.  It allows falling back to the historic rounding.
(org-time-inc): New helper function to increment date by Xm/d/w/m/y.
The new function, when `org-repeat-round-time' is non-nil, uses the
closest earlier existing calendar date when repeater units are month
or year.  Otherwise, it relies upon Emacs' rounding of non-existing
calendar dates being transferred to the next month's existing dates.
(org-timestamp-change): Use the new helper function.
(org-closest-date): Use the new helper function when computing
the expected closest repeater date.
* etc/ORG-NEWS (Repeater intervals in the units of month or year are
now computed as in many other calendar apps): Document the change.

Link: https://orgmode.org/list/87frvzodze.fsf@localhost
---
 etc/ORG-NEWS |  19 ++++++++
 lisp/org.el  | 127 ++++++++++++++++++++++++++++-----------------------
 2 files changed, 88 insertions(+), 58 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 87b72ad12..8f4e51734 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -13,6 +13,25 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
 
 * Version 9.7 (not released yet)
 ** Important announcements and breaking changes
+*** Repeater intervals in the units of month or year are now computed as in many other calendar apps
+
+Previously, timestamps like [2024-05-31 Fri +1m], when the next month
+does not have 31st day, were repeated to the first days of the
+following month: [2024-07-01 Mon +1m].  Same for years, when the same
+month next year does not have specified date.
+
+Now, the behavior is consistent with many common calendar apps - the
+closest /existing/ earlier date is selected: [2024-05-31 Fri +1m]
+repeats to [2024-06-30 Sun +1m].
+
+The previous behavior can be restored by customizing new option -
+~org-repeat-round-time~.
+
+Do note, however, that timestamps initially pointing to the last day
+of the month will not remain on the last day of the following months:
+[2024-05-31 Fri +1m] -> [2024-06-30 Sun +1m] -> [2024-07-30 Tue +1m]
+(not the last day anymore).
+
 *** ~org-create-file-search-functions~ can use ~org-list-store-props~ to suggest link description
 
 In Org <9.0, ~org-create-file-search-functions~ could set ~description~
diff --git a/lisp/org.el b/lisp/org.el
index 598b4ca23..81ac307cf 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -14951,7 +14951,7 @@ (defun org-diary-to-ical-string (frombuf)
     rtn))
 
 (defun org-closest-date (start current prefer)
-  "Return closest date to CURRENT starting from START.
+  "Return closest absolute date to CURRENT starting from START.
 
 CURRENT and START are both time stamps.
 
@@ -14961,12 +14961,19 @@ (defun org-closest-date (start current prefer)
 
 Only time stamps with a repeater are modified.  Any other time
 stamp stay unchanged.  In any case, return value is an absolute
-day number."
+day number.
+
+The return value is the number of days elapsed since the imaginary
+Gregorian date Sunday, December 31, 1 BC, as returned by
+`time-to-days'."
   (if (not (string-match "\\+\\([0-9]+\\)\\([hdwmy]\\)" start))
       ;; No repeater.  Do not shift time stamp.
       (time-to-days (org-time-string-to-time start))
-    (let ((value (string-to-number (match-string 1 start)))
-	  (type (match-string 2 start)))
+    (let* ((value (string-to-number (match-string 1 start)))
+	   (type (match-string 2 start))
+           (type-unit (pcase type
+                        ("h" 'hour) ("d" 'day) ("w" 'week)
+                        ("m" 'month) ("y" 'year))))
       (if (= 0 value)
 	  ;; Repeater with a 0-value is considered as void.
 	  (time-to-days (org-time-string-to-time start))
@@ -14993,50 +15000,17 @@ (defun org-closest-date (start current prefer)
 	       (let ((value (if (equal type "w") (* 7 value) value)))
 		 (setf n1 (+ sday (* value (/ (- cday sday) value))))
 		 (setf n2 (+ n1 value))))
-	      ("m"
-	       (let* ((add-months
-		       (lambda (d n)
-			 ;; Add N months to gregorian date D, i.e.,
-			 ;; a list (MONTH DAY YEAR).  Return a valid
-			 ;; gregorian date.
-			 (let ((m (+ (nth 0 d) n)))
-			   (list (mod m 12)
-				 (nth 1 d)
-				 (+ (/ m 12) (nth 2 d))))))
-		      (months		; Complete months to TARGET.
-		       (* (/ (+ (* 12 (- (nth 2 target) (nth 2 base)))
-				(- (nth 0 target) (nth 0 base))
-				;; If START's day is greater than
-				;; TARGET's, remove incomplete month.
-				(if (> (nth 1 target) (nth 1 base)) 0 -1))
-			     value)
-			  value))
-		      (before (funcall add-months base months)))
-		 (setf n1 (calendar-absolute-from-gregorian before))
-		 (setf n2
-		       (calendar-absolute-from-gregorian
-			(funcall add-months before value)))))
-	      (_
-	       (let* ((d (nth 1 base))
-		      (m (nth 0 base))
-		      (y (nth 2 base))
-		      (years		; Complete years to TARGET.
-		       (* (/ (- (nth 2 target)
-				y
-				;; If START's month and day are
-				;; greater than TARGET's, remove
-				;; incomplete year.
-				(if (or (> (nth 0 target) m)
-					(and (= (nth 0 target) m)
-					     (> (nth 1 target) d)))
-				    0
-				  1))
-			     value)
-			  value))
-		      (before (list m d (+ y years))))
-		 (setf n1 (calendar-absolute-from-gregorian before))
-		 (setf n2 (calendar-absolute-from-gregorian
-			   (list m d (+ (nth 2 before) value)))))))
+	      ((or "m" "y")
+	       (let* ((running-date (org-parse-time-string start))
+                      (next-date (org-time-inc type-unit value running-date))
+                      (current-date (org-parse-time-string current)))
+                 (while (not (time-less-p (org-encode-time current-date)
+                                        (org-encode-time next-date)))
+                   (setq running-date next-date
+                         next-date (org-time-inc type-unit value running-date)))
+                 (setf n1 (time-to-days (org-encode-time running-date))
+                       n2 (time-to-days (org-encode-time next-date)))))
+	      (_ (error "Unsupported repeater type: %S" type)))
 	    ;; Handle PREFER parameter, if any.
 	    (cond
 	     ((eq prefer 'past)   (if (= cday n2) n2 n1))
@@ -15193,6 +15167,52 @@ (defun org-at-clock-log-p ()
         (save-match-data (org-element-at-point))
         'clock)))
 
+(defcustom org-repeat-round-time t
+  "When non-nil, adjust repeated date down if it points to non-existing date.
+
+For example, when the repeater is monthly, this option, when non-nil,
+makes 31 May 2024 repeat to 30 June 2024 next month, adjusting the
+date down to avoid non-existent June 31st.  When nil, the same
+repeater would instead repeat the date at July 1st, retaining the
+extra day created by adding a monthly repeater."
+  :group 'org-time
+  :type 'boolean
+  :package-version '(Org . 9.7))
+
+(defun org-time-inc (unit value time)
+  "Increment TIME by VALUE UNITs and return new decoded time.
+TIME is decoded time, as returned by `decode-time'.
+VALUE is a number.  UNIT is one of symbols `second', `minute', `hour',
+`day', `month', `year'."
+  (unless (memq unit '(second minute hour day month year))
+    (error "org-time-inc: Unknown unit %S" unit))
+  (let ((new-time
+         (decode-time
+          (org-encode-time
+           (list
+            (+ (if (eq unit 'second) value 0) (decoded-time-second time))
+            (+ (if (eq unit 'minute) value 0) (decoded-time-minute time))
+            (+ (if (eq unit 'hour) value 0)   (decoded-time-hour time))
+            (+ (if (eq unit 'day) value 0)    (decoded-time-day time))
+            (+ (if (eq unit 'month) value 0)  (decoded-time-month time))
+            (+ (if (eq unit 'year) value 0)   (decoded-time-year time))
+            (decoded-time-weekday time)
+            (if (memq unit '(day month year))
+                nil ; Avoid auto-adjustments of time when jumping across DST.
+              (decoded-time-dst time))
+            (decoded-time-zone time))))))
+    (if (not org-repeat-round-time) new-time
+      (pcase unit
+        ((or `year `month)
+         (let ((target-year (when (eq unit 'year) (+ value (decoded-time-year time))))
+               (target-month (when (eq unit 'month) (+ value (decoded-time-month time)))))
+           (when (> target-month 12) (setq target-month (mod target-month 12)))
+           (while (or (and target-year (not (equal (decoded-time-year new-time) target-year)))
+                      (and target-month (not (equal (decoded-time-month new-time) target-month))))
+             (setq new-time (org-time-inc 'day -1 new-time)))
+           new-time))
+        (_ new-time)))))
+
 (defvar org-clock-history)                     ; defined in org-clock.el
 (defvar org-clock-adjust-closest nil)          ; defined in org-clock.el
 (defun org-timestamp-change (n &optional what updown suppress-tmp-delay)
@@ -15259,16 +15279,7 @@ (defun org-timestamp-change (n &optional what updown suppress-tmp-delay)
           ;; argument is supplied - just use whatever is provided by the
           ;; prefix argument.
           (setq dm 1))
-        (setq time
-	      (org-encode-time
-               (apply #'list
-                      (or (car time0) 0)
-                      (+ (if (eq timestamp? 'minute) increment 0) (nth 1 time0))
-                      (+ (if (eq timestamp? 'hour) increment 0)   (nth 2 time0))
-                      (+ (if (eq timestamp? 'day) increment 0)    (nth 3 time0))
-                      (+ (if (eq timestamp? 'month) increment 0)  (nth 4 time0))
-                      (+ (if (eq timestamp? 'year) increment 0)   (nth 5 time0))
-                      (nthcdr 6 time0)))))
+        (setq time (org-encode-time (org-time-inc timestamp? increment time0))))
       (when (and (memq timestamp? '(hour minute))
 		 extra
 		 (string-match "-\\([012][0-9]\\):\\([0-5][0-9]\\)" extra))
-- 
2.45.0


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-05-13 10:07   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
@ 2024-05-14 11:08     ` Max Nikulin
  2024-05-14 12:56       ` Ihor Radchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Max Nikulin @ 2024-05-14 11:08 UTC (permalink / raw)
  To: emacs-orgmode

On 13/05/2024 17:07, Ihor Radchenko wrote:
> 
> However, there are still some issues remaining.
> When updating timestamps repeating monthly across months with 30, 31,
> and 28 days we get
> 
> <2025-01-31 Fri +1m>
> <2025-02-28 Fri +1m>
> <2025-03-28 Fri +1m>
> ...
> <2026-01-28 Wed +1m>

Instead of using timestamp obtained on previous step, use original 
timestamp and multiple of the interval.

I have the following in my notes. However I have not find suitable 
functions in elisp:

https://debbugs.gnu.org/54764#10
Paul Eggert. Sat, 9 Apr 2022 00:52:57 -0700
> 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.

Do you have any idea what Paul was writing about?

On 13/05/2024 17:07, Ihor Radchenko wrote:
> +           (type-unit (pcase type
> +                        ("h" 'hour) ("d" 'day) ("w" 'week)
> +                        ("m" 'month) ("y" 'year))))
I do not mind against `pcase' here, I just expected something like 
`assoc' and `string-to-char'.

> +	      ((or "m" "y")
Another comment that may be ignored: `type-unit' values might be used here.

> +          (org-encode-time
> +           (list
> +            (+ (if (eq unit 'second) value 0) (decoded-time-second time))
> +            (+ (if (eq unit 'minute) value 0) (decoded-time-minute time))
> +            (+ (if (eq unit 'hour) value 0)   (decoded-time-hour time))
> +            (+ (if (eq unit 'day) value 0)    (decoded-time-day time))

Have you considered using `min' with result of `date-days-in-month' here 
(or its sibling from timezone.el)?

> +(defun org-time-inc (unit value time)
Is there a chance that support of intervals like 1h20m will be required 
later?

> +            (+ (if (eq unit 'month) value 0)  (decoded-time-month time))
> +            (+ (if (eq unit 'year) value 0)   (decoded-time-year time))
> +            (decoded-time-weekday time)
> +            (if (memq unit '(day month year))
> +                nil ; Avoid auto-adjustments of time when jumping across DST.

Sorry, but you have to use -1, otherwise

(format-time-string
  "%F %T %Z %z"
  (encode-time '(0 30 12 15 1 2000 'ignored nil "Africa/Juba"))
  "Africa/Juba")
(error "Specified time is not representable")

> +              (decoded-time-dst time))
> +            (decoded-time-zone time))))))
> +    (if (not org-repeat-round-time) new-time
I am in doubts if `org-time-inc' should access `org-repeat-round-time' 
directly or its value should be passed as an explicit argument. Perhaps 
the additional argument may be optional with fallback to 
`org-repeat-round-time' when it is omitted.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-05-14 11:08     ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
@ 2024-05-14 12:56       ` Ihor Radchenko
  2024-05-14 13:10         ` Stefan Nobis
  2024-05-15 11:04         ` Max Nikulin
  0 siblings, 2 replies; 12+ messages in thread
From: Ihor Radchenko @ 2024-05-14 12:56 UTC (permalink / raw)
  To: Max Nikulin, Adam Porter; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> writes:

> On 13/05/2024 17:07, Ihor Radchenko wrote:
>> 
>> However, there are still some issues remaining.
>> When updating timestamps repeating monthly across months with 30, 31,
>> and 28 days we get
>> 
>> <2025-01-31 Fri +1m>
>> <2025-02-28 Fri +1m>
>> <2025-03-28 Fri +1m>
>> ...
>> <2026-01-28 Wed +1m>
>
> Instead of using timestamp obtained on previous step, use original 
> timestamp and multiple of the interval.

This is not possible because of how `org-auto-repeat-maybe' is designed.
When repeater is triggered, the original date in the timestamp is
replaced with future date. So, we have no access to the original date in
the timestamp.

> I have the following in my notes. However I have not find suitable 
> functions in elisp:
>
> https://debbugs.gnu.org/54764#10
> Paul Eggert. Sat, 9 Apr 2022 00:52:57 -0700
>> 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.
>
> Do you have any idea what Paul was writing about?

No idea. I am not aware about any existing built-in API that provides
date increments.

>> +          (org-encode-time
>> +           (list
>> +            (+ (if (eq unit 'second) value 0) (decoded-time-second time))
>> +            (+ (if (eq unit 'minute) value 0) (decoded-time-minute time))
>> +            (+ (if (eq unit 'hour) value 0)   (decoded-time-hour time))
>> +            (+ (if (eq unit 'day) value 0)    (decoded-time-day time))
>
> Have you considered using `min' with result of `date-days-in-month' here 
> (or its sibling from timezone.el)?

Not sure if it is going to be simpler.

>> +(defun org-time-inc (unit value time)
> Is there a chance that support of intervals like 1h20m will be required 
> later?

Not sure again. I simply used ts-inc function signature from Adam's
ts.el as reference.

>> +            (+ (if (eq unit 'month) value 0)  (decoded-time-month time))
>> +            (+ (if (eq unit 'year) value 0)   (decoded-time-year time))
>> +            (decoded-time-weekday time)
>> +            (if (memq unit '(day month year))
>> +                nil ; Avoid auto-adjustments of time when jumping across DST.
>
> Sorry, but you have to use -1, otherwise
>
> (format-time-string
>   "%F %T %Z %z"
>   (encode-time '(0 30 12 15 1 2000 'ignored nil "Africa/Juba"))
>   "Africa/Juba")
> (error "Specified time is not representable")

Hmm. I am not sure if it is a real problem in practice.
String values of time zone are only possible for hand-crafted time
values. Using `decode-time' does not retain the time zone name:

(decode-time
  (encode-time '(0 30 12 15 1 2000 'ignored -1 "Africa/Juba")))
; => (0 30 12 15 1 2000 6 nil 7200)

The reason why I forced DST nil is hysteresis of glibc when dealing with
DST transitions:

(cl-loop for m in '(1 2 3 4 5 6 7 8 9 10 11 12)
  collect (decode-time (encode-time `(0 0 0 4 ,m 2012 nil -1 10800))))

(0 0 23 3 1 2012 2 nil 7200)
(0 0 23 3 2 2012 5 nil 7200)
(0 0 23 3 3 2012 6 nil 7200)
(0 0 0  4 4 2012 3 t 10800)
(0 0 0  4 5 2012 5 t 10800)
(0 0 0  4 6 2012 1 t 10800)
(0 0 0  4 7 2012 3 t 10800)
(0 0 0  4 8 2012 6 t 10800)
(0 0 0  4 9 2012 2 t 10800)
(0 0 0  4 10 2012 4 t 10800)
(0 0 23 3 11 2012 6 nil 7200)
(0 0 23 3 12 2012 1 nil 7200)

Although, forcing DST nil will not always help here, so I need some
other solution to this problem.
(timezones, DSTs... :sigh:)

>> +              (decoded-time-dst time))
>> +            (decoded-time-zone time))))))
>> +    (if (not org-repeat-round-time) new-time
> I am in doubts if `org-time-inc' should access `org-repeat-round-time' 
> directly or its value should be passed as an explicit argument. Perhaps 
> the additional argument may be optional with fallback to 
> `org-repeat-round-time' when it is omitted.

This is of no use for now - we do not need to pass different parameters
to `org-time-inc' from the existing Org code. But we can always add an
extra optional argument if necessary.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-05-14 12:56       ` Ihor Radchenko
@ 2024-05-14 13:10         ` Stefan Nobis
  2024-05-15 11:04         ` Max Nikulin
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Nobis @ 2024-05-14 13:10 UTC (permalink / raw)
  To: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> This is not possible because of how `org-auto-repeat-maybe' is
> designed. When repeater is triggered, the original date in the
> timestamp is replaced with future date. So, we have no access to the
> original date in the timestamp.

And even if we would have access to the original date: It may be
something like the last day of February and from that alone it is not
evident, wether e.g. the 28. of each month should be used or the last
day of month.

Therefore, I think, the only solution to this problem is to have some
kind of special 'last day of month' marker. Maybe something like
"+1m!" or any other way of encoding this "go to the last day of the
next month" directly into the increment specifier.

-- 
Until the next mail...,
Stefan.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?)
  2024-05-14 12:56       ` Ihor Radchenko
  2024-05-14 13:10         ` Stefan Nobis
@ 2024-05-15 11:04         ` Max Nikulin
  1 sibling, 0 replies; 12+ messages in thread
From: Max Nikulin @ 2024-05-15 11:04 UTC (permalink / raw)
  To: emacs-orgmode

On 14/05/2024 19:56, Ihor Radchenko wrote:
> Max Nikulin writes:
>> On 13/05/2024 17:07, Ihor Radchenko wrote:
>>>
>>> <2025-01-31 Fri +1m>
>>> <2025-02-28 Fri +1m>
>>> <2025-03-28 Fri +1m>
>>
>> Instead of using timestamp obtained on previous step, use original
>> timestamp and multiple of the interval.
> 
> This is not possible because of how `org-auto-repeat-maybe' is designed.

Then the only way to get reasonable results is to store in :PROPERTIES: 
either original date (and iteration count) or current date before 
clamping (2025-02-31)

> No idea. I am not aware about any existing built-in API that provides
> date increments.

The context was that day start time may give some surprise as well. I am 
unsure concerning namely increments. I just expected a set of more 
accurate functions for working with dates.

>>> +          (org-encode-time
[...]
>>> +            (+ (if (eq unit 'day) value 0)    (decoded-time-day time))
>>
>> Have you considered using `min' with result of `date-days-in-month' here
>> (or its sibling from timezone.el)?
> 
> Not sure if it is going to be simpler.

My idea was to avoid the backward `while' loop in the code below this line.

>> (format-time-string
>>    "%F %T %Z %z"
>>    (encode-time '(0 30 12 15 1 2000 'ignored nil "Africa/Juba"))
>>    "Africa/Juba")
>> (error "Specified time is not representable")
> 
> Hmm. I am not sure if it is a real problem in practice.
> String values of time zone are only possible for hand-crafted time
> values.

This example avoids changing system time or starting Emacs with TZ 
environment. The following example does not include explicit timezone:

TZ=Africa/Juba emacs -Q --batch \
     --eval "(encode-time '(0 30 12 15 1 2000 'ignored nil nil))"

> The reason why I forced DST nil is hysteresis of glibc when dealing with
> DST transitions:

At first I was trying to figure out what time zone had 24:00<->23:00 
transition in 2012.

> (cl-loop for m in '(1 2 3 4 5 6 7 8 9 10 11 12)
>    collect (decode-time (encode-time `(0 0 0 4 ,m 2012 nil -1 10800))))
> 
> (0 0 23 3 1 2012 2 nil 7200)

This result depends on your timezone. It is consistent with e.g. 
Asia/Istanbul. It had +02:00 offset in January, but you requested 
+03:00. The result of conversion is -1 hour difference in requested 
timestamp and conversion result. It is unrelated to internal DST state 
and hysteresis caused by this state.

When DST is nil
2012-01-04 00:00:00 +03:00 === 2012-01:03 23:00:00 +02:00

Avoid passing timezone offset when you do not know it.

I agree that there are issues with handling tm_gmtoff and tm_isdst
https://data.iana.org/time-zones/theory.html#POSIX-extensions
Results may vary across libraries.

> Although, forcing DST nil will not always help here

It is fragile, I would not rely on ignoring offset when DST is 
specified. Actually you pass mutually inconsistent values, so it is 
either undefined behavior or close to it.




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-05-15 11:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-29 10:48 Leap-year bug with todo-cycle Anton Haglund
2024-04-05 18:34 ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
2024-04-05 19:53   ` Russell Adams
2024-04-05 21:18   ` jman
2024-04-05 21:27     ` Ihor Radchenko
2024-04-06 14:52   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
2024-04-07 11:47     ` Ihor Radchenko
2024-05-13 10:07   ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) (was: Leap-year bug with todo-cycle) Ihor Radchenko
2024-05-14 11:08     ` [POLL] Dealing with +1m/y repeaters when jumping to impossible date (should 05-31 +1m be 07-01 or 06-30?) Max Nikulin
2024-05-14 12:56       ` Ihor Radchenko
2024-05-14 13:10         ` Stefan Nobis
2024-05-15 11:04         ` Max Nikulin

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).